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

Fixing some issues with indexing sets in generic properties #339

Merged
merged 1 commit into from
May 12, 2021

Conversation

andrewlee94
Copy link
Member

@andrewlee94 andrewlee94 commented May 12, 2021

Fixes #338

Summary/Motivation:

Cleans up a missing check for non-vapourisable and non-condensable components, plus updating to the new API for indexing sets.

Changes proposed in this PR:

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.

@andrewlee94 andrewlee94 self-assigned this May 12, 2021
@andrewlee94 andrewlee94 added bug Something isn't working Priority:High High Priority Issue or PR property packages Issues dealing with properties labels May 12, 2021
@andrewlee94 andrewlee94 added this to In progress in 2021 May idaes-pse Release via automation May 12, 2021
@andrewlee94 andrewlee94 moved this from In progress to Review in progress in 2021 May idaes-pse Release May 12, 2021
Copy link
Member

@eslickj eslickj 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 as far as I can tell.

Copy link
Contributor

@agarciadiego agarciadiego left a comment

Choose a reason for hiding this comment

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

LGTM

2021 May idaes-pse Release automation moved this from Review in progress to Reviewer approved May 12, 2021
@codecov-commenter
Copy link

Codecov Report

Merging #339 (09e77d5) into main (9b494aa) will decrease coverage by 0.09%.
The diff coverage is 60.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #339      +/-   ##
==========================================
- Coverage   64.34%   64.25%   -0.10%     
==========================================
  Files         324      324              
  Lines       52050    52044       -6     
  Branches     9162     9162              
==========================================
- Hits        33490    33439      -51     
- Misses      16905    16983      +78     
+ Partials     1655     1622      -33     
Impacted Files Coverage Δ
idaes/generic_models/properties/core/eos/ceos.py 82.55% <50.00%> (ø)
...models/properties/core/generic/generic_property.py 65.47% <63.63%> (ø)
idaes/conftest.py 80.00% <0.00%> (-20.00%) ⬇️
idaes/dmf/util.py 54.29% <0.00%> (-16.75%) ⬇️
idaes/ver.py 61.53% <0.00%> (-4.62%) ⬇️
idaes/dmf/codesearch.py 98.55% <0.00%> (-1.45%) ⬇️
idaes/surrogate/pysmo/kriging.py 80.56% <0.00%> (-0.71%) ⬇️
idaes/core/process_base.py 87.62% <0.00%> (-0.13%) ⬇️
.../properties/interrogator/reactions_interrogator.py 91.74% <0.00%> (-0.08%) ⬇️
idaes/logger.py 90.20% <0.00%> (-0.07%) ⬇️
... and 9 more

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 9b494aa...09e77d5. Read the comment docs.

@andrewlee94 andrewlee94 merged commit d113fec into IDAES:main May 12, 2021
2021 May idaes-pse Release automation moved this from Reviewer approved to Done May 12, 2021
@andrewlee94 andrewlee94 deleted the issue_338 branch May 13, 2021 13:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working Priority:High High Priority Issue or PR property packages Issues dealing with properties
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

Generic properties components not is phase
4 participants