Skip to content

Improve check valve validation and setting #487

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

Closed
wants to merge 4 commits into from

Conversation

angusmcb
Copy link
Contributor

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

Summary

Include issues that are resolved by this pull request.
Improved method of setting check_valve on pipes. Validation and conversion from string values 'True', 'False', '1' and '0' occurs within element rather than with add_pipe. This means when check_valve is set directly validation will occur.

Contributres to #422. This is also a step towards allowing use of geopandas >= 1.0. This is because shapefile booleans seem to be returned as strings 'True' and 'False' rather than strings '1' and '0' so current normalisation with check_valve = bool(int(check_valve)) fails. This resolves the following error when trying to use geopandas >= 1.0

>       check_valve = bool(int(check_valve))
E       ValueError: invalid literal for int() with base 10: 'False'

May also fix #475

Tests and documentation

Does not change the API functionality, covered by existing tests.

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.

@angusmcb
Copy link
Contributor Author

I believe the test fails are not related to the changes made

@angusmcb
Copy link
Contributor Author

Closing in favour of #488 which contains these changes but is more complete allowing the latest geopandas to be used.

@angusmcb angusmcb closed this Mar 13, 2025
@coveralls
Copy link

Coverage Status

coverage: 81.805% (+0.02%) from 81.788%
when pulling 0584c2e on angusmcb:improve-check_valve-validation
into 88ca1d9 on USEPA:main.

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.

basic_demo.ipynb check_valve must be a Boolean
2 participants