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

Xarray accessor updates to remove automatic quantification and in-place operations #1430

Merged
merged 4 commits into from Jul 29, 2020

Conversation

jthielen
Copy link
Collaborator

Description Of Changes

Update of #1378 on new fork and after rebase on latest master. Original comment below:

Based on lingering discussions (particularly in #1354), this PR implements a few cleanups on the xarray accessors needed for work on #1353 to continue, namely, removing automatic conversion to quantities in parse_cf (since its loading of data into memory can cause errors) and making sure all operations are not in-place. There is one place where "side effects" still occur (caching of the coordinate parsing results in the _metpy_axis attribute), but I figured this was still okay (and hard to avoid, since it is invoked when using a property).

Checklist

@jthielen jthielen added Type: Maintenance Updates and clean ups (but not wrong) Area: Xarray Pertains to xarray integration labels Jul 28, 2020
@jthielen jthielen added this to the 1.0 milestone Jul 28, 2020
@jthielen
Copy link
Collaborator Author

I realized that this uses xr.Dataset.map, which was introduced in v0.14.1 (Nov 2019). Should we bump minimum version from v0.13.0 (Sep 2019), or should I have a fallback to xr.Dataset.apply, which is pending deprecation?

dopplershift
dopplershift previously approved these changes Jul 29, 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.

This looks good. I like the extensive additions in notes in the docs and comments.

@dopplershift
Copy link
Member

Eh, what's another two months shorter on our xarray horizon? Go ahead and bump. Would just 0.14.0 work? (Could always let our tests determine that.) If not 0.14.1's fine.

Hey does that mean we can work around some of those deprecated functions now? 😄

@dopplershift
Copy link
Member

Answered my own question here as I did some digging for other reasons. We need 0.14.1 for xr.Dataset.map, so that's fine. 0.14.1 was a weird release for xarray in that it includes new "features".

@jthielen
Copy link
Collaborator Author

jthielen commented Jul 29, 2020

Answered my own question here as I did some digging for other reasons. We need 0.14.1 for xr.Dataset.map, so that's fine. 0.14.1 was a weird release for xarray in that it includes new "features".

Yeah, xarray hasn't been the best at following semver...their "patch" releases sometimes have new APIs and breaking changes.

Hey does that mean we can work around some of those deprecated functions now? 😄

I'll take a look after this PR!

@dopplershift
Copy link
Member

Not sure why Travis shows queued, it's passed. Codacy is...well, wrong.

@dopplershift dopplershift merged commit 615a3c6 into Unidata:master Jul 29, 2020
@jthielen
Copy link
Collaborator Author

So now it's mostly just the updated version of #1353 to go before 1.0-rc2?

@dopplershift
Copy link
Member

Sounds great. I promise this one will actually get my attention. Thanks for knocking these out. I'm really happy with how CI is working with these.

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.

How does cross_section recognize projections?
2 participants