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

BUG: fix future incompatiblities with unyt 3.0 (2/n) #4166

Merged
merged 2 commits into from
Oct 13, 2022

Conversation

neutrinoceros
Copy link
Member

@neutrinoceros neutrinoceros commented Oct 13, 2022

PR Summary

Follow up to #4136 and yt-project/unyt#293, this adapts yt to stay compatible with latest development in unyt (#4162).

Here's my reasoning:

  • some of unyt's API is now marked as deprecated (on the dev branch). Specifically, all unit-preserving numpy wrapper functions (e.g. unyt.udot and unyt.uconcatenate)
  • we cannot yet migrate to the vanilla numpy API (e.g., np.dot and np.concatenate) that will be the replacement because that will not work before unyt 3.0 is available.
  • both the yt and yt.units namespace currently re-expose this unyt API directly

The simpler option I can see to get around the deprecation warnings in CI without simply filtering them is to
vendor specifically this part of unyt's API, but do so in an isolated and hidden module (yt.units._numpy_wrapper_functions), and use that internally. This module is meant to be deprecated itself at some point in the future, but we'll be able to control that on yt's own development cycle, instead of being tightly coupled to unyt's.

I ran the bleeding-edge workflow on my fork to show that this branch fixes the current instabilities: https://github.com/neutrinoceros/yt/actions/runs/3241090204

I'm labelling this as 2/n because there will be more changes upstream soon ( we're making numpy API unit-aware), but this is by far the largest PR of this series. Upcoming PRs should be much smaller in size and look more like the second commit of this branch.

@neutrinoceros neutrinoceros added this to the 4.1.1 milestone Oct 13, 2022
@neutrinoceros neutrinoceros marked this pull request as ready for review October 13, 2022 12:08
@neutrinoceros
Copy link
Member Author

@jzuhone can you have a look please ?

@jzuhone
Copy link
Contributor

jzuhone commented Oct 13, 2022

@neutrinoceros Can you just leave a bit more explanation in the description as to what this is doing and why? I think I get it but for future reference we should probably have that information.

@neutrinoceros
Copy link
Member Author

@jzuhone better ?

@jzuhone jzuhone merged commit 0c840ad into yt-project:main Oct 13, 2022
meeseeksmachine pushed a commit to meeseeksmachine/yt that referenced this pull request Oct 13, 2022
@neutrinoceros neutrinoceros deleted the unyt_3.0_compat_2 branch October 13, 2022 16:15
neutrinoceros added a commit that referenced this pull request Oct 14, 2022
…6-on-yt-4.1.x

Backport PR #4166 on branch yt-4.1.x (BUG: fix future incompatiblities with unyt 3.0 (2/n))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants