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

Fixes for attributes of coordinates #140

Merged
merged 6 commits into from
Sep 11, 2020
Merged

Fixes for attributes of coordinates #140

merged 6 commits into from
Sep 11, 2020

Conversation

johnomotani
Copy link
Collaborator

@johnomotani johnomotani commented Sep 10, 2020

A bug in xarray (pydata/xarray#2180 (comment), pydata/xarray#4393, pydata/xarray#4415) can result in attributes being deleted from coordinates when a new DataArray is added to a Dataset. This PR adds workarounds for that bug, and also some other fixes to make sure re-loaded Datasets get coordinates identical to the saved Dataset (including attrs).

Also added a work-around for a second weird possible bug in xarray pydata/xarray#4417.

We should not read or alter ds after updated_ds is created, so delete
the reference to ds to avoid errors.
@johnomotani johnomotani added the bugfix Fix for a bug label Sep 10, 2020
@pep8speaks
Copy link

pep8speaks commented Sep 10, 2020

Hello @johnomotani! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2020-09-10 23:14:44 UTC

An xarray bug
(pydata/xarray#2180 (comment),
pydata/xarray#4393,
pydata/xarray#4415) can deletes attributes of
coordinates when a new DataArray is assigned to a Dataset. This commit
adds workarounds for that bug in several places.

Also adds checks for if 'ycoord' and 'zcoord' already exist in
apply_geometry() before creating them. Helps with re-applying
apply_geometry() to existing or re-loaded Datasets.
Workaround for possible bug with xarray,
pydata/xarray#4417, needed for ensuring
consistency so we can test for 'identical'.
Using assign_coords() can change existing coordinates on other
variables, use a workaround instead to ensure more consistency.
Ensures input is not modified.
@codecov-commenter
Copy link

codecov-commenter commented Sep 11, 2020

Codecov Report

Merging #140 into master will increase coverage by 0.06%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #140      +/-   ##
==========================================
+ Coverage   77.62%   77.69%   +0.06%     
==========================================
  Files          14       14              
  Lines        2150     2156       +6     
  Branches      486      487       +1     
==========================================
+ Hits         1669     1675       +6     
  Misses        308      308              
  Partials      173      173              
Impacted Files Coverage Δ
xbout/geometries.py 79.16% <100.00%> (+0.29%) ⬆️
xbout/region.py 87.44% <100.00%> (+0.11%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 747fe2a...3a990fa. Read the comment docs.

@johnomotani johnomotani merged commit 04c5c0d into master Sep 11, 2020
@johnomotani johnomotani deleted the fix-coord-attrs branch September 11, 2020 14:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix Fix for a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants