Skip to content

Conversation

@blnicho
Copy link
Member

@blnicho blnicho commented Oct 17, 2025

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-assignment and possibly-used-before-assignment. To fix the warnings I used a combination of code refactoring or skipping the checks.

Changes proposed in this PR:

  • Update to pylint 3.3.9
  • Fix or disable new pylint warnings

Legal Acknowledgement

By contributing to this software project, I agree to the following terms and conditions for my contribution:

  1. I agree my contributions are submitted under the license terms described in the LICENSE.txt file at the top level of this directory.
  2. I represent I am authorized to make the contributions and grant the license. If my employer has rights to intellectual property that includes these contributions, I represent that I have received permission to make contributions and grant the required license on behalf of that employer.

@codecov
Copy link

codecov bot commented Oct 17, 2025

Codecov Report

❌ Patch coverage is 94.11765% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 77.07%. Comparing base (ce7f3bd) to head (37538e3).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
...d_contactors/unit_models/bubbling_fluidized_bed.py 66.66% 1 Missing ⚠️
...a/gas_solid_contactors/unit_models/fixed_bed_1D.py 50.00% 0 Missing and 1 partial ⚠️
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.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@dallan-keylogic dallan-keylogic added the CI:run-integration triggers_workflow: Integration label Oct 17, 2025
@idaes-build idaes-build removed the CI:run-integration triggers_workflow: Integration label Oct 17, 2025
@ksbeattie ksbeattie added the Priority:High High Priority Issue or PR label Oct 23, 2025
@blnicho blnicho changed the title Bumping the Pyomo tag to check for issues Update pylint to 3.3.9 Oct 23, 2025
@blnicho blnicho added the CI:run-integration triggers_workflow: Integration label Oct 24, 2025
@idaes-build idaes-build removed the CI:run-integration triggers_workflow: Integration label Oct 24, 2025
@blnicho blnicho added the CI:run-integration triggers_workflow: Integration label Oct 24, 2025
@idaes-build idaes-build removed the CI:run-integration triggers_workflow: Integration label Oct 24, 2025
@blnicho blnicho requested a review from mrmundt October 24, 2025 00:10
Copy link
Contributor

@dallan-keylogic dallan-keylogic left a 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 .
Copy link
Contributor

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?

Copy link
Member Author

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.

Comment on lines +343 to +349
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),
Copy link
Contributor

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.

Copy link
Member Author

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.

@ksbeattie ksbeattie merged commit 7ec0e54 into IDAES:main Oct 30, 2025
168 of 204 checks passed
sufikaur pushed a commit to sufikaur/idaes-pse that referenced this pull request Nov 17, 2025
* 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Priority:High High Priority Issue or PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants