-
Notifications
You must be signed in to change notification settings - Fork 6
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
Source priority input #2022
base: main
Are you sure you want to change the base?
Source priority input #2022
Conversation
python/ribasim/ribasim/input_base.py
Outdated
for n in range(1, len(names_lowered) + 1): | ||
node_name_snake_case = "_".join(names_lowered[:n]) | ||
if node_name_snake_case in node_names_snake_case: | ||
node_name = "".join(names[:n]) | ||
table_name = "_".join(names_lowered[n:]) | ||
return node_name + delimiter + table_name | ||
return names[0] |
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.
This doesn't work properly yet (in the sense that it doesn't produce a proper table name for my new table), but the python tests failed on this because my new table name neither contains a node name nor is it one word long
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.
Can you describe the design in this PR? From what I remember we need a configurable priority per nodetype only? And if we needed to override that (not sure), there can be a column in the Node table.
Given that context, why do we need a python/ribasim/ribasim/allocation/source_priorities.py
?
default_source_priority_user_demand = 1000 # optional, default 1000 | ||
default_source_priority_boundary = 2000 # optional, default 2000 | ||
default_source_priority_level_demand = 3000 # optional, default 3000 | ||
default_source_priority_flow_demand = 4000 # optional, default 4000 | ||
default_source_priority_subnetwork_inlet = 5000 # optional, default 5000 |
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.
I would make it a substructure here, called source_priority
or similar, and removing default
from the name.
It's still nice to summarize what the changes are in a PR description (and later commit message). Can be very short, as in rename |
Fixes #1505.
To do:
priority -> demand_priority
a non-breaking migration step