Skip to content

Conversation

eminyouskn
Copy link
Contributor

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.

Copy link
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.

from collections import defaultdict


class _Hasher(defaultdict):
Copy link
Member

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
Contributor Author

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.

Copy link

codecov bot commented Aug 27, 2025

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.

Copy link
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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants