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

feat: Add a parent namespace to Parameters. #234

Merged
merged 3 commits into from
Aug 7, 2024

Conversation

jetuk
Copy link
Member

@jetuk jetuk commented Jul 30, 2024

Core Parameters can now be identified with an optional parent string. This is helpful for creating Parameters in compound nodes without risking naming conflicts with the user's own parameter names.

Fixes #179.

Core Parameters can now be identified with an optional parent
string. This is helpful for creating Parameters in compound
nodes without risking naming conflicts with the user's own
parameter names.

Fixes #179.
@jetuk jetuk requested a review from Batch21 July 30, 2024 13:25
@@ -207,13 +208,13 @@ impl LoadedTimeseriesCollection {
.ok_or(TimeseriesError::TimeseriesNotFound(name.to_string()))?;

let array: Array2<f64> = df.to_ndarray::<Float64Type>(IndexOrder::default()).unwrap();
let name = format!("timeseries.{}_{}", name, scenario);
Copy link
Member Author

Choose a reason for hiding this comment

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

Was this "timeseries" prefix required for some reason?

Copy link
Contributor

Choose a reason for hiding this comment

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

only to miminise the risk of the name clashing with an existing non-timeseries parameter

Copy link
Contributor

@Batch21 Batch21 left a comment

Choose a reason for hiding this comment

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

LGTM

@jetuk jetuk merged commit f28360c into main Aug 7, 2024
7 checks passed
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.

Add parameter name spaces
2 participants