-
Notifications
You must be signed in to change notification settings - Fork 4
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
feat(python): Add conversion helper functions to Python extension. #231
Conversation
New helper functions for converting from Pywr v1.x to v2.x JSON strings added to the Python extension. Includes a new wrapper around `Metric`.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code looks good but I'm getting an import error for the new functions when running this locally.
I should just have to run maturin develop
to install it into a venv right?
Cargo.toml
Outdated
@@ -47,5 +47,6 @@ tracing = { version = "0.1", features = ["log"] } | |||
csv = "1.1" | |||
hdf5 = { git = "https://github.com/aldanor/hdf5-rust.git", package = "hdf5", features = ["static", "zlib"] } | |||
pywr-v1-schema = "0.14" | |||
# pywr-v1-schema = { path = "/home/james/dev/pywr/pywr-schema/pywr-v1-schema" } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove this ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
pywr-schema/src/nodes/core.rs
Outdated
@@ -831,7 +831,7 @@ pub struct AggregatedStorageNode { | |||
} | |||
|
|||
impl AggregatedStorageNode { | |||
const DEFAULT_ATTRIBUTE: NodeAttribute = NodeAttribute::Outflow; | |||
const DEFAULT_ATTRIBUTE: NodeAttribute = NodeAttribute::Volume; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be fixed in another PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See #238
I think so? I guess I should add some Python tests for these functions. |
deleted the target directory and recreated the venv and it worked. Probably worth adding some more extensive python tests at some point but I tested both functions with some of the test models and they worked fine for me. |
New helper functions for converting from Pywr v1.x to v2.x JSON strings added to the Python extension. Includes a new wrapper around
Metric
.