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

Create demo_config.yml #83

Merged
merged 18 commits into from
Mar 18, 2024
Merged

Create demo_config.yml #83

merged 18 commits into from
Mar 18, 2024

Conversation

barneydobson
Copy link
Collaborator

@barneydobson barneydobson commented Mar 13, 2024

Description

PR to implement configuration file, used to specify what to call from swmmanywhere

Fixes #9 #10 #19

In part #28 - though switching extension is not yet included - this is listed under #28

Notes:

  • swmmanywhere function (is this a suitable name? conscious that it could confuse people's imports) reads a config file and calls the various functions, including downloads (if necessary), graphfcns, run, and metrics.
  • Added a demo_config.yml, currently used in testing
  • A bit of various debugging/improvmenets from running everything all together (there's a few find+replace here which has led to many new lines).
  • New functions (iterate_graphfcns and iterate_metrics) iterate over both registers, currently within the respective utilities modules.
  • object -> id in results/metrics
  • assign_id is now called at the start and end of graphfcn_list, since SWMM cannot have edge names overlapping with node names (which in turn occurs due to split_long_edges - now highlighted in Refactor split_long_edges #39 )

This PR will not address:

@barneydobson barneydobson linked an issue Mar 13, 2024 that may be closed by this pull request
@barneydobson barneydobson self-assigned this Mar 13, 2024
Copy link
Collaborator

@dalonsoa dalonsoa left a comment

Choose a reason for hiding this comment

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

It's looking ok, but I'm very concerned with the lack of validation of the configuration.

swmmanywhere/graph_utilities.py Show resolved Hide resolved
swmmanywhere/metric_utilities.py Outdated Show resolved Hide resolved
swmmanywhere/metric_utilities.py Show resolved Hide resolved
swmmanywhere/swmmanywhere.py Outdated Show resolved Hide resolved
swmmanywhere/swmmanywhere.py Outdated Show resolved Hide resolved
swmmanywhere/swmmanywhere.py Show resolved Hide resolved
swmmanywhere/swmmanywhere.py Outdated Show resolved Hide resolved
swmmanywhere/swmmanywhere.py Outdated Show resolved Hide resolved
swmmanywhere/swmmanywhere.py Outdated Show resolved Hide resolved
swmmanywhere/swmmanywhere.py Show resolved Hide resolved
Dobson and others added 6 commits March 15, 2024 09:24
Co-authored-by: Diego Alonso Álvarez <6095790+dalonsoa@users.noreply.github.com>
Co-authored-by: Diego Alonso Álvarez <6095790+dalonsoa@users.noreply.github.com>
remove config load from swmmanywhere.swmmanywhere
@barneydobson barneydobson requested a review from dalonsoa March 15, 2024 11:22
@barneydobson
Copy link
Collaborator Author

barneydobson commented Mar 15, 2024

It's looking ok, but I'm very concerned with the lack of validation of the configuration.

@dalonsoa

Thanks for the suggestions - if you wanted to specifically check the now added schema.yml and expanded load_config that would be amazing

Copy link
Collaborator

@dalonsoa dalonsoa left a comment

Choose a reason for hiding this comment

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

This looks much better. You should add a dedicated test for load_config (not to test what jsonshema is doing, but what the other bits and pieces are doing, but otherwise it looks good now.

swmmanywhere/defs/schema.yml Outdated Show resolved Hide resolved
tests/test_swmmanywhere.py Show resolved Hide resolved
swmmanywhere/graph_utilities.py Outdated Show resolved Hide resolved
swmmanywhere/metric_utilities.py Outdated Show resolved Hide resolved
swmmanywhere/swmmanywhere.py Show resolved Hide resolved
swmmanywhere/graph_utilities.py Outdated Show resolved Hide resolved
swmmanywhere/graph_utilities.py Show resolved Hide resolved
tests/test_swmmanywhere.py Show resolved Hide resolved
This was referenced Mar 15, 2024
@barneydobson barneydobson merged commit 63af329 into main Mar 18, 2024
10 checks passed
@barneydobson barneydobson deleted the 10-configuration-file branch March 18, 2024 09:25
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.

Configuration file File/address validation
3 participants