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

Fixed float parsing in clean_osm_data.py #1089

Merged
merged 3 commits into from
Sep 6, 2024

Conversation

rsparks3
Copy link
Contributor

@rsparks3 rsparks3 commented Aug 18, 2024

Closes #1087 .

Changes proposed in this Pull Request

Changes _parse_float to use try/except instead of trying to detect the type and return based on that. The previous function did not accept inputs such as "3.0" because the python .isnumeric() function will return false.

Checklist

  • I consent to the release of this PR's code under the AGPLv3 license and non-code contributions under CC0-1.0 and CC-BY-4.0.
  • I tested my contribution locally and it seems to work fine.
  • Code and workflow changes are sufficiently documented.
  • Newly introduced dependencies are added to envs/environment.yaml and doc/requirements.txt.
  • Changes in configuration options are added in all of config.default.yaml and config.tutorial.yaml.
  • Add a test config or line additions to test/ (note tests are changing the config.tutorial.yaml)
  • Changes in configuration options are also documented in doc/configtables/*.csv and line references are adjusted in doc/configuration.rst and doc/tutorial.rst.
  • A note for the release notes doc/release_notes.rst is amended in the format of previous release notes, including reference to the requested PR.

Copy link
Member

@davide-f davide-f left a comment

Choose a reason for hiding this comment

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

Hello @rsparks3!
The formulation looks quite good to me!
Have you tested the impact into the tutorial?
In particular the difference between the older and new version?

Feel free to add a release note too

@rsparks3
Copy link
Contributor Author

rsparks3 commented Sep 5, 2024

Thanks! The tutorial runs fine and produces similar results as the older version, but I have not closely inspected to see how many additional lines become a part of the finished networks. With the whole continent of Africa, however, it seemed to add about ~50-100 more lines.

@davide-f
Copy link
Member

davide-f commented Sep 6, 2024

Thanks! The tutorial runs fine and produces similar results as the older version, but I have not closely inspected to see how many additional lines become a part of the finished networks. With the whole continent of Africa, however, it seemed to add about ~50-100 more lines.

Great! That is realistic and acceptable.
Merging as checks complete :)

@davide-f davide-f merged commit 182f7dd into pypsa-meets-earth:main Sep 6, 2024
5 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.

_parse_float does not appropriately handle all inputs in clean_osm_data.py
2 participants