Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #407 +/- ##
==========================================
+ Coverage 88.10% 88.27% +0.16%
==========================================
Files 23 23
Lines 2287 2337 +50
Branches 288 294 +6
==========================================
+ Hits 2015 2063 +48
- Misses 193 194 +1
- Partials 79 80 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a new I/O structure for SWMM file processing by replacing the old synthetic_write function with a more modular swmmio‐based approach and additional testing.
- Implement new I/O functions registered through a decorator and managed by an io_registry.
- Add a custom parameter group and update configuration schemas to support custom I/O modules.
- Update tests to validate model construction and I/O registration.
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_post_processing.py | Added new tests and fixtures to verify the updated post processing using swmmio. |
| tests/test_data/custom_io.py | Introduces a custom I/O function registered via the decorator. |
| src/swmmanywhere/swmmanywhere.py | Adds a new function for registering custom I/O modules and integrates it into config loading. |
| src/swmmanywhere/post_processing.py | Implements I/O registry, new I/O functions (e.g., apply_nodes, iterate_io) and a helper method. |
| src/swmmanywhere/parameters.py | Adds a new parameter group for post processing. |
| src/swmmanywhere/defs/schema.yml | Updates the schema to include custom_io_modules. |
| pyproject.toml | Adds the swmmio dependency. |
Comments suppressed due to low confidence (1)
src/swmmanywhere/post_processing.py:79
- Reference to 'gpd' is used without being imported. Please add 'import geopandas as gpd' at the top of the file.
nodes = gpd.read_file(addresses.model_paths.nodes)
|
The main issue is that |
|
Ah OK thanks for raising that. For now it will be to mimic the old behaviour - so perhaps there can be optional imports/behaviour? I can also just delay this - it's not urgent. It's for a new case study I'm working on, but I can equally keep that code separate |
|
I guess it is not possible to have a |
|
They don’t seem to plan on adding it to EDIT: It seems that we're back on track with |
|
Yep, for what it seems in the issue you open, the look quite enthusiastic and open to do so. That's good news. |
dalonsoa
left a comment
There was a problem hiding this comment.
This looks good, but I have a couple of comments. More important, I'm not entirely sure what's the advantage of this over what you had before. Could you elaborate? It might be part of the documentation, as well.
| nodes["id"] = nodes["id"].astype(str) | ||
| nodes["max_depth"] = nodes.surface_elevation - nodes.chamber_floor_elevation | ||
| nodes["surcharge_depth"] = 0 | ||
| nodes["flooded_area"] = 100 # TODO arbitrary... not sure how to calc this |
There was a problem hiding this comment.
Maybe add an issue about it, if you haven't done so, yet.
There was a problem hiding this comment.
Yeah I have just moved these to the 'default SWMM parameters' yaml instead, as they make more sense there for now. I guess ultimately everything will be moved to be a SWMManywhere parameter in parameters.py, but that will be in a different PR
Co-authored-by: Diego Alonso Álvarez <6095790+dalonsoa@users.noreply.github.com>
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
Description
This PR proposes the general structure for switching from my hacky SWMM file writing functions to
swmmio(a devoted package for SWMM file IO). This also facilitates easy customisation of SWMM model file writing. Theiterate_iofunction, when complete, will replacesynthetic_write.I keep the SWMManywhere design philosophy of:
If this looks good - I will convert the rest of
post_processinginto this format and switch over to useiterate_ioin the mainswmmanywherecall. Once approved, subsequent PRs on the topic will be to merge into this branch.Fixes: #84
Type of change
Key checklist
python -m pytest)python -m sphinx -b html docs docs/build)pre-commit run --all-files)Further checks