Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Fix example's BasicUnit array conversion.#19535

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
tacaswell merged 3 commits intomatplotlib:masterfromQuLogic:fix-units-old-np
Mar 25, 2021

Conversation

QuLogic
Copy link
Member

@QuLogicQuLogic commentedFeb 18, 2021•
edited
Loading

PR Summary

The unit examples are broken in NumPy < 1.20. This fixes a bug that was exposed by#19415, but does not fix the examples fully.radian_demo is fixed by this change, butellipse_with_units is still broken. I do not understand the NumPy subclassing methods enough to fix it short of reverting#19415..

A unit is a scalar, not a length-1 array. ThoughBasicUnit implements__rmul__, if multiplying by an array, the NumPy implementation will call__array__ instead. If the LHS is an array, everything is fine, but if the LHS is a scalar, the previous code would incorrectly cause it to be upcast to a 1D array. When__getitem__ was added in#19415,np.atleast_1d started iterating each (now 1D, not scalar)TaggedValue, seeing it was length 1, and made the x/y arrays into (N, 1) instead of (N,).

PR Checklist

  • [n/a] Has pytest style unit tests (andpytest passes).
  • IsFlake 8 compliant (runflake8 on changed files to check).
  • [n/a] New features are documented, with examples if plot related.
  • Documentation is sphinx and numpydoc compliant (the docs shouldbuild without error).
  • Conforms to Matplotlib style conventions (installflake8-docstrings and runflake8 --docstring-convention=all).
  • [n/a] New features have an entry indoc/users/next_whats_new/ (follow instructions in README.rst there).
  • [n/a] API changes documented indoc/api/next_api_changes/ (follow instructions in README.rst there).

@QuLogic
Copy link
MemberAuthor

QuLogic commentedFeb 18, 2021•
edited
Loading

The remaining failure is:

$python examples/units/ellipse_with_units.py Traceback (most recent call last):  File "examples/units/ellipse_with_units.py", line 34, in <module>    x += xcenterTypeError: unsupported operand type(s) for +: 'TaggedValue' and 'float'

I am using NumPy 1.18.1/1.18.5. It appears to have fixed itself in 1.19.0.@dstansby

@tacaswell
Copy link
Member

tacaswell commentedFeb 19, 2021•
edited
Loading

It seems that our attempts to use a minimum version of numpy is getting foiled on cricle and we are running the minimum version docs build with np 1.20 🤦 .

@QuLogic
Copy link
MemberAuthor

Good catch; hopefully this should work to fix that.

A unit is a scalar, not a length-1 array. Though `BasicUnit` implements`__rmul__`, if multiplying by an array, the NumPy implementation willcall `__array__` instead. If the LHS is an array, everything is fine,but if the LHS is a scalar, the previous code would incorrectly cause itto be upcast to a 1D array. When `__getitem__` was added inmatplotlib#19415,`np.atleast_1d` started iterating each (now 1D, not scalar)`TaggedValue`, seeing it was length 1, and made the x/y arrays into(N, 1) instead of (N,).
On older versions (1.18), this breaks things, so must be skipped.
@jklymak
Copy link
Member

I looked to see what this example does, but it doesn't do anything except register itself?

@tacaswelltacaswell merged commit1fdc2cd intomatplotlib:masterMar 25, 2021
@lumberbot-app
Copy link

Owee, I'm MrMeeseeks, Look at me.

There seem to be a conflict, please backport manually. Here are approximate instructions:

  1. Checkout backport branch and update it.
$ git checkout v3.4.x$ git pull
  1. Cherry pick the first parent branch of the this PR on top of the older branch:
$ git cherry-pick -m1 1fdc2cda42f642a09dea727ba058efa75dd5cc2e
  1. You will likely have some merge/cherry-pick conflict here, fix them and commit:
$ git commit -am "Backport PR #19535: Fix example's BasicUnit array conversion."
  1. Push to a named branch :
git push YOURFORK v3.4.x:auto-backport-of-pr-19535-on-v3.4.x
  1. Create a PR against branch v3.4.x, I would have named this PR:

"Backport PR#19535 on branch v3.4.x"

And apply the correct labels and milestones.

Congratulation you did some good work ! Hopefully your backport PR will be tested by the continuous integration and merged soon!

If these instruction are inaccurate, feel free tosuggest an improvement.

@QuLogicQuLogic deleted the fix-units-old-np branchMarch 25, 2021 19:31
@QuLogic
Copy link
MemberAuthor

I looked to see what this example does, but it doesn't do anything except register itself?

It's imported by all the other examples in this directory.

@QuLogic
Copy link
MemberAuthor

With other backports done, I hope this will work now.

@meeseeksdev backport to v3.4.x

meeseeksmachine pushed a commit to meeseeksmachine/matplotlib that referenced this pull requestMar 25, 2021
QuLogic added a commit that referenced this pull requestMar 25, 2021
…535-on-v3.4.xBackport PR#19535 on branch v3.4.x (Fix example's BasicUnit array conversion.)
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@tacaswelltacaswelltacaswell approved these changes

Assignees
No one assigned
Projects
None yet
Milestone
v3.4.0
Development

Successfully merging this pull request may close these issues.

3 participants
@QuLogic@tacaswell@jklymak
[8]ページ先頭
©2009-2025 Movatter.jp