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

Add support for locstream lon/lat in bbox and shape subsetting #288

Merged
merged 3 commits into from
Jun 27, 2023

Conversation

aulemahal
Copy link
Collaborator

Pull Request Checklist:

  • This PR addresses an already opened issue (for bug fixes / features)
    • This PR fixes issue #xyz
  • Tests for the changes have been added (for bug fixes / features)
  • Documentation has been added / updated (for bug fixes / features)
  • HISTORY.rst has been updated (with summary of main changes)
  • I have added my relevant user information to AUTHORS.md
  • What kind of change does this PR introduce?:
    Adds the case for a dataset where the lon and lat coords are 1D and share a dimension. This is the case for station data, for example. "locstream" is the word used in ESMF, I didn't have a better idea.

The changes are located in mostly subset_bbox, but it also required a small change in create_masks so that subset_shape would work too.

In subset_bbox, for the curvilinear case, I also removed drop=True from the code. It is not doing anything because we are already clipping to the smallest box above, but more importantly, if it was doing anything it would fail in the dataset case. Some var would have coords dropped and be added back in the dataset, where the full coords still exist.

@coveralls
Copy link

coveralls commented Jun 23, 2023

Pull Request Test Coverage Report for Build 5392226100

  • 6 of 6 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.05%) to 78.455%

Totals Coverage Status
Change from base Build 5391770466: 0.05%
Covered Lines: 1107
Relevant Lines: 1411

💛 - Coveralls

Copy link
Collaborator

@Zeitsperre Zeitsperre left a comment

Choose a reason for hiding this comment

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

Clean. Excited to try this out.

@aulemahal aulemahal merged commit a1f619a into master Jun 27, 2023
@aulemahal aulemahal deleted the locstream-support branch June 27, 2023 17:18
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