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.

Sign up for GitHub

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

Jump to bottom

swmmnetwork allows for mixed dtypes in self #4

Closed
lucashtnguyen opened this issue Oct 6, 2017 · 1 comment
Closed

swmmnetwork allows for mixed dtypes in self #4

lucashtnguyen opened this issue Oct 6, 2017 · 1 comment

Comments

@lucashtnguyen
Copy link
Contributor

While iterating thorugh the nodes in line156 these can be mixed types. This causes line222 to fail if the node is an int.

This is ultimately an upstream issue with ScenarioLoading but I think that SwmmNetwork needs to also enforce string types so that if any([i in node for i in self.outfall_flags]) does not fail.
lucashtnguyen pushed a commit to lucashtnguyen/swmmnetwork that referenced this issue Oct 6, 2017
scenario.py gets a test suite, yay!
9e4bd31 
I modified `scenario.py` and `swmmnetwork.py` to hopefully
close austinorr#4. I added a small swmm model .inp and .rpt to run all future
tests on. This test suite is not yet complete, but it is a good start.
@lucashtnguyen lucashtnguyen mentioned this issue Oct 6, 2017
Closed
@austinorr
Copy link
Owner

In SwmmNetwork, node id's are allowed to be int, but id names read from the SWMM5.1 report file and/or input file should all be strings. I believe this is addressed fully by #5 and a few tweaks to how the id's are split into volume reduced flags, or treatment flags, so I'll close this. I have written a unit test that completes as I expect with int and string types in the SwmmNetwork object, and we'll also add a test for files loaded via the scenario builder objects.

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

scenario.py gets a test suite, yay! lucashtnguyen/swmmnetwork
2 participants
@lucashtnguyen @austinorr