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

Support crs file for observation cross sections #289

Merged

Conversation

tim-vd-aardweg
Copy link
Contributor

No description provided.

tim-vd-aardweg and others added 27 commits July 11, 2022 14:24
…ons' of https://github.com/Deltares/HYDROLIB-core into feature/159_Support_CrsFile_for_observation_cross_sections

# Conflicts:
#	hydrolib/core/io/obscrosssection/models.py
#	tests/io/test_obscrosssection.py
Added an additional validator to the ObservationPointCrossSection class.
…ons' of https://github.com/Deltares/HYDROLIB-core into feature/159_Support_CrsFile_for_observation_cross_sections

# Conflicts:
#	hydrolib/core/io/obscrosssection/models.py
#	tests/io/test_obscrosssection.py
…nates match the expecter number of coordinates.
…ons' of https://github.com/Deltares/HYDROLIB-core into feature/159_Support_CrsFile_for_observation_cross_sections

# Conflicts:
#	hydrolib/core/io/ini/util.py
@priscavdsluis priscavdsluis linked an issue Jul 19, 2022 that may be closed by this pull request
"""

general: ObservationPointCrossSectionGeneral = ObservationPointCrossSectionGeneral()
crosssections: List[ObservationPointCrossSection] = []
Copy link
Contributor

Choose a reason for hiding this comment

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

The agreed convention for these lists is to call the field the singular form of the object in the list, so in this case:

Suggested change
crosssections: List[ObservationPointCrossSection] = []
observationpointcrosssection: List[ObservationPointCrossSection] = []

(but it should be observationcrosssection)

I don't remember why exactly, but I will ask @arthurvd or @BeardedPlatypus when they get back.
There was a specific reason for it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

Copy link
Contributor

@priscavdsluis priscavdsluis left a comment

Choose a reason for hiding this comment

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

Look really nice and well documented!
I do have some remarks. Please take a look and let me know what you think.
There are also 2 code smells, The one about the complexicity ties together with one comment I wrote and the other I think we can ignore, since it has to do with the way we use pydantic validation.

@tim-vd-aardweg tim-vd-aardweg changed the title Support crs file for observation point cross sections Support crs file for observation cross sections Jul 26, 2022
@sonarcloud
Copy link

sonarcloud bot commented Aug 18, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

No Coverage information No Coverage information
0.0% 0.0% Duplication

@priscavdsluis priscavdsluis merged commit f1d0096 into main Aug 18, 2022
@priscavdsluis priscavdsluis deleted the feature/159_Support_CrsFile_for_observation_cross_sections branch August 18, 2022 06:57
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.

Support CrsFile for observation cross sections
2 participants