Revision of OLI Cloud Flash Calculations #1237
Conversation
exception test for get_credentials() Co-authored-by: Adam Atia <[email protected]>
|
|
||
| :return boolean: True on success, False on failure | ||
| :return status: bool indicating success or failure |
There was a problem hiding this comment.
Should "refresh" be in the doc string (even though it is self-explanatory)?
There was a problem hiding this comment.
Yes, I just missed it
There was a problem hiding this comment.
@veccp Can you add it? This PR is pretty much ready to go but this is a super minor revision that should be handled in this PR.
There was a problem hiding this comment.
Yes, I just pushed the change
|
The low reported patch coverage is expected: see #1249 for details. |
There was a problem hiding this comment.
LGTM - I think the only outstanding comment was a minor docstring issue, and even though I asked @veccp to add it in this PR, I don't mind just leaving it for a subsequent PR so that we don't have to wait for all tests to rerun in this PR.
Oh- we'll have to wait for tests to rerun anyway since we have to merge main. I'll leave it open. |
|
@adam-a-a I was waiting for us to test this against OLI before merging. |
Fixes/Resolves:
Provides a framework for adding additional OLI Cloud flash calculations as the need emerges. Currently supporting Water Analysis and Isothermal Analysis.
Summary/Motivation:
Prior analyses used bespoke scripts to make flash calculations and extract data; this approach simplifies the process for the user
Changes proposed in this PR:
Legal Acknowledgement
By contributing to this software project, I agree to the following terms and conditions for my contribution: