Skip to content
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

_component_data_iter now uses sorted_robust #1852

Merged
merged 6 commits into from
Mar 10, 2021

Conversation

PierreAttard
Copy link
Contributor

@PierreAttard PierreAttard commented Mar 5, 2021

Fixes # .

Fix #1845

Summary/Motivation:

This resolves an issue with sorted_robust reported by @emma58 in #1845.
Block._component_data_iter does not use the correction did @jsiirola in the PR #1842. I just add his correction to the current PR in order to solve #1845.

Changes proposed in this PR:

  • Extend the sorted_robust API to accept key= and reverse= arguments
  • import sorted_robust in block.py and sort with sorted_robust instead of six.itemgetter

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.

@codecov
Copy link

codecov bot commented Mar 8, 2021

Codecov Report

Merging #1852 (45632e6) into main (e6ed67e) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #1852   +/-   ##
=======================================
  Coverage   79.69%   79.69%           
=======================================
  Files         560      560           
  Lines       69822    69828    +6     
=======================================
+ Hits        55646    55652    +6     
  Misses      14176    14176           
Impacted Files Coverage Δ
pyomo/core/base/block.py 90.61% <100.00%> (+0.01%) ⬆️
pyomo/core/base/misc.py 82.06% <100.00%> (+0.64%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e6ed67e...45632e6. Read the comment docs.

jsiirola added a commit to jsiirola/pyomo that referenced this pull request Mar 9, 2021
@jsiirola
Copy link
Member

jsiirola commented Mar 9, 2021

We definitely need to move to using sorted_robust, but we need to preserve the use of itemgetter(0) so that component data objects are not accidentally compared. There is a PR to this PR (PierreAttard#1) that extends the sorted_robust API to support key= and reverse= arguments and adds tests. If/once merged, then this PR should be ready to go.

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.

Looks good, pending successful tests

@PierreAttard
Copy link
Contributor Author

Is that possible to re-run a specific test ?

Win3.6 and 3.8 tests fails because of that :

image

@jsiirola
Copy link
Member

jsiirola commented Mar 9, 2021

Unfortunately, no. GHA only supports "rerun everything". We will wait to see if everything else passes, and if it does, I would be comfortable merging even with those failures (as they are unrelated to this PR).

@jsiirola jsiirola merged commit 27fc7d0 into Pyomo:main Mar 10, 2021
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.

_component_data_iter I think doesn't use sorted_robust?
4 participants