Skip to content

Improving properties metadata #995

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.

Sign up for GitHub

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

Jump to bottom
Merged
andrewlee94 merged 57 commits into from
Jan 20, 2023
Merged

Improving properties metadata #995

andrewlee94 merged 57 commits into from
Jan 20, 2023

Conversation

@andrewlee94
Copy link
Contributor

@andrewlee94 andrewlee94 commented Oct 24, 2022

Part of #669

Summary/Motivation:

This PR introduces a new PropertyMetadata and PropertySet object for recording what thermophysical properties are supported by a property package. The eventual goal is to have a concrete definition of the standard properties used in IDAES that can be accessed programmatically and does not rely on string constants, as well as formalizing what metadata is stored for each.

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.

Andrew Lee added 18 commits October 17, 2022 10:17
@andrewlee94 andrewlee94 self-assigned this Oct 24, 2022
@andrewlee94 andrewlee94 added enhancement New feature or request Priority:Normal Normal Priority Issue or PR IDAES v2.0 labels Oct 24, 2022
@andrewlee94
Copy link
Contributor Author

andrewlee94 commented Oct 24, 2022

Note that test failures are expected for now - there are some tests that are using the property metadata for unexpected purposes.

@codecov
Copy link

codecov bot commented Oct 25, 2022 •
edited
Loading

Codecov Report

Base: 70.06% // Head: 70.18% // Increases project coverage by +0.11% 🎉

Coverage data is based on head (dd0298a) compared to base (feed2ea).
Patch coverage: 89.89% of modified lines in pull request are covered.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main     #995      +/-   ##
==========================================
+ Coverage   70.06%   70.18%   +0.11%     
==========================================
  Files         397      399       +2     
  Lines       65077    65318     +241     
  Branches    12058    12089      +31     
==========================================
+ Hits        45597    45844     +247     
- Misses      17131    17132       +1     
+ Partials     2349     2342       -7
Impacted Files Coverage Δ
.../models_extra/gas_distribution/unit_models/node.py 90.37% <0.00%> (ø)
...els_extra/gas_distribution/unit_models/pipeline.py 94.40% <0.00%> (ø)
...d_contactors/unit_models/bubbling_fluidized_bed.py 73.67% <ø> (ø)
.../activity_coeff_models/activity_coeff_prop_pack.py 83.36% <61.11%> (-0.04%) ⬇️
idaes/core/base/util.py 77.94% <77.94%> (ø)
...ties/methane_iron_OC_reduction/gas_phase_thermo.py 82.44% <81.81%> (ø)
...rties/oxygen_iron_OC_oxidation/gas_phase_thermo.py 79.95% <81.81%> (ø)
idaes/core/base/property_meta.py 94.44% <82.45%> (-2.39%) ⬇️
idaes/core/base/property_set.py 96.98% <96.98%> (ø)
idaes/core/__init__.py 100.00% <100.00%> (ø)
... and 14 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

jsiirola
jsiirola reviewed Dec 5, 2022
View reviewed changes
jsiirola
jsiirola approved these changes Jan 17, 2023
View reviewed changes
idaes/core/base/property_set.py
if meth is None:
super().__setattr__("_method", None)
else:
super().__setattr__("_method", str(meth))
Copy link
Contributor

@jsiirola jsiirola Dec 19, 2022

Choose a reason for hiding this comment

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

Why str(meth)?

@andrewlee94 andrewlee94 enabled auto-merge (squash) January 19, 2023 15:14
@andrewlee94 andrewlee94 mentioned this pull request Jan 19, 2023
Merged
@andrewlee94 andrewlee94 disabled auto-merge January 20, 2023 15:34
@andrewlee94 andrewlee94 enabled auto-merge (squash) January 20, 2023 16:43
@andrewlee94 andrewlee94 merged commit 0f1e266 into IDAES:main Jan 20, 2023
@andrewlee94 andrewlee94 deleted the issue_669_4 branch January 20, 2023 18:29
This was referenced Jan 20, 2023
Closed
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dangunter dangunter dangunter left review comments

@jsiirola jsiirola jsiirola approved these changes

+1 more reviewer

@eslickj eslickj eslickj approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@andrewlee94 andrewlee94

Labels

enhancement New feature or request IDAES v2.0 Priority:Normal Normal Priority Issue or PR

Projects

No open projects
2023 February Release
Status: Done

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@andrewlee94 @ksbeattie @jsiirola @dangunter @eslickj