-
Notifications
You must be signed in to change notification settings - Fork 292
Update pylint to 3.3.9 #1683
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update pylint to 3.3.9 #1683
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1683 +/- ##
==========================================
- Coverage 77.08% 77.07% -0.01%
==========================================
Files 395 395
Lines 62759 62786 +27
Branches 10234 10231 -3
==========================================
+ Hits 48377 48395 +18
- Misses 11974 11983 +9
Partials 2408 2408 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
dallan-keylogic
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
| - name: Run Black to verify that the committed code is formatted | ||
| run: | | ||
| black --check . | ||
| black --check --diff . |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the idea that we need to run black on only the files that have changes in them?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The --diff command will display the changes that black would like to make. It can be convenient to fix 1 character issues reported by black directly in the GitHub interface without having to run black locally.
| elif block._flow_direction == FlowDirection.forward: | ||
| _idx = block.length_domain.last() | ||
|
|
||
| port, ref_name_list = sblock.build_port( | ||
| doc, slice_index=(slice(None), _idx) | ||
| doc, | ||
| # pylint: disable-next=possibly-used-before-assignment | ||
| slice_index=(slice(None), _idx), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ideally, in a situation like this, we would have an else clause that raises BurntToast (even if CodeCov doesn't like us adding code that is theoretically inaccessible). Skipping the Pylint checks is a good solution for now, though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Exactly! There were several instances where I skipped the pylint check because I wasn't sure of a "safe" default value and I wanted to make sure an error was thrown if the enum options ever changed.
* Bumping the Pyomo tag to check for issues * Bump required versions of pylint, astroid, and black * Reverting update of black version * Changing pylint and astroid versions again * Update expected pylint and astroid versions * Fixing pylint warnings * Revert change to required Pyomo version in setup.py * Tell black to report the changes it wants * Run black
Summary/Motivation:
The Pyomo 6.9.5 release broke pylint testing in IDAES which is pinned to a very old version of pylint. The easiest fix is to bump the version of pylint to something more recent. This PR bumps pylint to 3.3.9 and fixes new pylint failures stemming from new checks that have been introduced namely
used-before-assignmentandpossibly-used-before-assignment. To fix the warnings I used a combination of code refactoring or skipping the checks.Changes proposed in this PR:
Legal Acknowledgement
By contributing to this software project, I agree to the following terms and conditions for my contribution: