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

Remove delr and delc from ds #346

Merged
merged 4 commits into from
Jun 17, 2024
Merged

Remove delr and delc from ds #346

merged 4 commits into from
Jun 17, 2024

Conversation

rubencalje
Copy link
Collaborator

@rubencalje rubencalje commented May 17, 2024

This pull request remove delr and delc from ds. When needed, these variables can be calculated from extend, x and y. Also see issue #343.

@rubencalje rubencalje changed the title Remove delr and delc from ds, first commit Remove delr and delc from ds May 17, 2024
@rubencalje rubencalje marked this pull request as ready for review June 10, 2024 13:21
Copy link
Collaborator

@OnnoEbbens OnnoEbbens left a comment

Choose a reason for hiding this comment

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

Nice! Only some small comments from me

@@ -92,6 +92,30 @@ def get_xy_mid_structured(extent, delr, delc, descending_y=True):
raise TypeError("unexpected type for delr and/or delc")


def get_delr(ds):
"""Get the distance along rows (delr) fromythe x-coordinate of a model dataset"""
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you make a full docstring with in-and output types?

"""Get the distance along columns (delr) from the x-coordinate of a model dataset"""
assert ds.gridtype == "structured"
y = (ds.extent[3] - ds.y).values
delc = _get_delr_from_x(y)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Kind of counterintuitive to call _get_delr_from_x to get_delc_from_y. Maybe rename the private method to _get_delta_along_axis or something similar?

@rubencalje rubencalje merged commit 6ff7172 into dev Jun 17, 2024
2 of 3 checks passed
@rubencalje rubencalje deleted the remove_delr_delc_from_dataset branch July 5, 2024 13:08
@dbrakenhoff dbrakenhoff mentioned this pull request Sep 19, 2024
1 task
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.

2 participants