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

Fix to nan Values Breaking UGRID Data Type and Fill Values #259

Merged
merged 6 commits into from
Apr 2, 2023

Conversation

philipc2
Copy link
Member

@philipc2 philipc2 commented Mar 29, 2023

Overview

  • Implementation from Standardized Fill Values and Data Types for Connectivity Variables #241 broke when nan was used for Mesh2_face_nodes, since np.isnan needs to be used to locate any occurrences of nan
  • This hotfix modifies test_standardized_dtype_and_fill to separate datasets that contain a fill value and those that don't
  • Changes were made to _read_ugrid to correctly parse the original fill value
  • _replace_fill_values now uses np.isnan to find all occurrences of nan

@philipc2 philipc2 self-assigned this Mar 29, 2023
@philipc2 philipc2 changed the title Fix to nan Values Breaking UGRID Data Type and Fill Values URGENT: Fix to nan Values Breaking UGRID Data Type and Fill Values Mar 29, 2023
@philipc2 philipc2 linked an issue Mar 29, 2023 that may be closed by this pull request
@philipc2 philipc2 changed the title URGENT: Fix to nan Values Breaking UGRID Data Type and Fill Values Fix to nan Values Breaking UGRID Data Type and Fill Values Mar 29, 2023
@philipc2
Copy link
Member Author

@erogluorhan @anissa111 @rajeeja

This is a hotfix PR to the issue @hongyuchen1030 discovered in #191. Hoping we can get this merged ASAP so that it doesn't inhibit her work.

@philipc2 philipc2 requested a review from hCraker March 29, 2023 16:49
uxarray/helpers.py Outdated Show resolved Hide resolved
Copy link
Contributor

@hongyuchen1030 hongyuchen1030 left a comment

Choose a reason for hiding this comment

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

I think this fix looks good in general! Thank you for your work! Just left some little questions/comments that I am not clear with.

uxarray/helpers.py Show resolved Hide resolved
uxarray/helpers.py Outdated Show resolved Hide resolved
test/test_ugrid.py Show resolved Hide resolved
test/test_ugrid.py Show resolved Hide resolved
test/test_ugrid.py Show resolved Hide resolved
@philipc2 philipc2 requested a review from hongyuchen1030 March 30, 2023 16:42
@philipc2
Copy link
Member Author

Documentation failing due to an issue upstream, can ignore for now (see pydata/pydata-sphinx-theme#1274)

@anissa111
Copy link
Member

Documentation failing due to an issue upstream, can ignore for now (see pydata/pydata-sphinx-theme#1274)

It looks like the sphinx-book-theme made a release last night (v1.0.1). I'd try re-running the docs without the pin now

@philipc2 philipc2 merged commit 7b68a25 into main Apr 2, 2023
@erogluorhan erogluorhan deleted the philipc2/fix-nan branch June 9, 2023 21:42
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.

Standardized Fill Values Break with nan Original Fill Values
3 participants