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

Fix bug when to_gis() is called on a network with a leak #458

Merged
merged 11 commits into from
Nov 19, 2024

Conversation

kbonney
Copy link
Collaborator

@kbonney kbonney commented Nov 13, 2024

This PR addresses the bug reported in #456.

Summary

  • Add logic to the to_dict method for controls so that the case when the target object is a Node can be handled.
  • Add the ability to process "stringified" controls in _read_control_line for Nodes. Previously only link elements were expected.
  • Add leak attributes to dictionary representation of nodes.
    • This requires forcibly deleting these attribute columns during the creation of node dataframes.
  • Read in leak attributes of Nodes during from_dict

Tests and documentation

A test case is added that runs to_dict on a network with a leak and verifies that the leak controls and attributes persist after the dictionary representation is read back in with from_dict.

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.

junction_name = wn.junction_name_list[0]
junction = wn.get_node(junction_name)
junction.add_leak(wn, area=1, start_time=0, end_time=3600)
wn.to_dict()
Copy link
Collaborator

Choose a reason for hiding this comment

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

I suggest adding from_dict to make sure the leak can be recreated in a water network model.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good idea!

@coveralls
Copy link

Coverage Status

coverage: 84.364% (+0.05%) from 84.319%
when pulling b7af055 on kbonney:leak_controls
into 3d5d970 on USEPA:main.

@kaklise kaklise requested a review from dbhart November 19, 2024 15:36
Copy link
Collaborator

@dbhart dbhart left a comment

Choose a reason for hiding this comment

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

These changes all look good.

@kaklise kaklise merged commit c5cfcbd into USEPA:main Nov 19, 2024
41 checks passed
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.

4 participants