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

Adding vertices to morph.node #332

Merged
merged 11 commits into from Feb 15, 2023
Merged

Adding vertices to morph.node #332

merged 11 commits into from Feb 15, 2023

Conversation

kbonney
Copy link
Collaborator

@kbonney kbonney commented Feb 8, 2023

Provide a summary of the proposed changes, describe tests and documentation, and review the acknowledgement below.

Summary

This PR resolves issue #283 by including link vertices in the transformations performed on node coordinates.

Tests and documentation

No new features were added, but new lines have been added to existing tests to cover vertex cases.

Acknowledgement

By contributing to this software project, I acknowledge that I have reviewed the software quality assurance guidelines and that my contributions are submitted under the Revised BSD License.

@kaklise kaklise self-requested a review February 13, 2023 21:22
@kbonney
Copy link
Collaborator Author

kbonney commented Feb 15, 2023

Updates related to test_epanet_io.py

The setup for this class has been modified so that the comparisons are made between a wn generated from the original file inp and one generated from the inp file created from a call to write_inpfile.
Doing this updates caused a couple hiccups in the subsequent tests:

  • When reading from the [DEMANDS] section of an inp file, wntr was assigning the category attribute to an empty string rather than None. Since demands are recorded directly to the junctions in write_inpfile, the second network would record the category attribute using different logic. In this case they would come out as None instead. I updated the read method to record empty strings as None, which seems to be more aligned with wntr's design. See wntr/epanet/io.py for exact changes
  • In the _compare method for the Link class, the initial_setting looked for exact equality. This caused issue since the read method truncates decimals to 9 places. I added logic to account for this. This required also taking care of the cases where one or the other value in the comparison was None, while the other was numeric. See wntr/network/base.py for more.
  • Since temp.inp is written in "LPM" rather than the original "GPM", this caused an issue when comparing hydraulic options. I added a line in the setup for test_epanet_io.py to manually change the hydraulic option for inpfile unit.

Copy link
Collaborator

@kaklise kaklise 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 good to me. Thanks for catching the issue in the epanet io test!

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.

None yet

2 participants