Skip to content

Clean pyomo collections module#3708

Merged
mrmundt merged 9 commits intoPyomo:mainfrom
eminyouskn:ye/collections
Aug 27, 2025
Merged

Clean pyomo collections module#3708
mrmundt merged 9 commits intoPyomo:mainfrom
eminyouskn:ye/collections

Conversation

@eminyouskn
Copy link
Copy Markdown
Contributor

@eminyouskn eminyouskn commented Aug 22, 2025

Fixes NA

Summary/Motivation:

This PR introduce some refactoring of the pyomo collections module.

Changes proposed in this PR:

  • Create a separate file for the hasher.
  • Remove OrderDict import from orderset.
  • Sort imports.

Legal Acknowledgement

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

  1. I agree my contributions are submitted under the BSD license.
  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.

jsiirola
jsiirola approved these changes Aug 27, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@jsiirola jsiirola left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can merge this, but it would be good to add a bit of documentation while we are refactoring.

pyomo/common/collections/_hasher.py Outdated
from collections import defaultdict


class _Hasher(defaultdict):
Copy link
Copy Markdown
Member

@jsiirola jsiirola Aug 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We talked about this on the dev call, but I think it would be worth removing the _ -- Sphinx by default will not document private classes, attributes, etc.

Of course, we should also add documentation to this class...

Copy link
Copy Markdown
Contributor Author

@eminyouskn eminyouskn Aug 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks John for the review. I added a short documentation of the class. I let you tell me if it is clear enough.

@codecov
Copy link
Copy Markdown

codecov bot commented Aug 27, 2025 •
edited
Loading

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.19%. Comparing base (7deb912) to head (1d92091).
⚠️ Report is 182 commits behind head on main.

Additional details and impacted files 
@@           Coverage Diff           @@
##             main    #3708   +/-   ##
=======================================
  Coverage   89.19%   89.19%           
=======================================
  Files         892      893    +1     
  Lines      103100   103099    -1     
=======================================
  Hits        91956    91956           
+ Misses      11144    11143    -1
Flag Coverage Δ
builders 26.73% <65.15%> (+<0.01%) ⬆️
default 85.79% <100.00%> (?)
expensive 34.07% <80.30%> (?)
linux 86.95% <100.00%> (-1.97%) ⬇️
linux_other 86.95% <100.00%> (-0.01%) ⬇️
osx 83.29% <100.00%> (-0.01%) ⬇️
win 85.17% <100.00%> (+<0.01%) ⬆️
win_other 85.17% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ 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.

jsiirola
jsiirola reviewed Aug 27, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@jsiirola jsiirola left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Some minor wording changes (the only real reason for the change is to line wrap, and to us the Sphinx "magic comment" syntax)

@mrmundt mrmundt merged commit 04b8a5f into Pyomo:main Aug 27, 2025
1 of 6 checks passed
@eminyouskn eminyouskn deleted the ye/collections branch August 27, 2025 19:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jsiirola jsiirola jsiirola approved these changes

@mrmundt mrmundt mrmundt approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@eminyouskn @jsiirola @mrmundt @pyomo-autotest