Skip to content

Refactor parse_cf to not modify coordinates inplace#1285

Merged
dopplershift merged 2 commits intoUnidata:masterfrom
jthielen:parse-cf-refactor
Jan 12, 2020
Merged

Refactor parse_cf to not modify coordinates inplace#1285
dopplershift merged 2 commits intoUnidata:masterfrom
jthielen:parse-cf-refactor

Conversation

@jthielen
Copy link
Collaborator

Description Of Changes

Refactors parse_cf to not modify coordinates (which may be tied to immutable indexes) inplace, and instead reassign the coordinates in the recommended way. If this goes as my tests locally lead me to believe, this should fix the issues created with xarray in pydata/xarray#3519.

Checklist

@dopplershift dopplershift added Area: Xarray Pertains to xarray integration Type: Maintenance Updates and clean ups (but not wrong) labels Jan 10, 2020
@dopplershift dopplershift added this to the 1.0 milestone Jan 10, 2020
dopplershift
dopplershift previously approved these changes Jan 12, 2020
Copy link
Member

@dopplershift dopplershift left a comment

Choose a reason for hiding this comment

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

I like the implementation approach taken here. Very clean to assemble the new coords and assign once.

@dopplershift
Copy link
Member

I'll rebase and merge.

@dopplershift
Copy link
Member

Oh, no this was already up to date. The test failure was real, failing with our oldest supported xarray. It looks like assign_coords() only recent accepted passing a dict-like, and before only accepted **kwargs. Looks like an easy tweak that should work across versions. I've pushed a commit to fix. Hopefully this passes.

@dopplershift
Copy link
Member

Ignoring Codacy and merging.

@dopplershift dopplershift merged commit ce72871 into Unidata:master Jan 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: Xarray Pertains to xarray integration Type: Maintenance Updates and clean ups (but not wrong)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Issues with XArray master

2 participants