Allow lists of quantities to be converted in Matplotlib unit converter by dstansby · Pull Request #7037 · astropy/astropy

dstansby · 2017-12-27T20:21:09Z

Quite a lot of the unit support built into Matplotlib, and quite a lot of the internals, works on lists of unit-like object. This lets a list of quantities be converted fine using the Matplotlib unit converters in Astropy.

As an example, this change makes vlines and hlines work in Matplotlib:

from astropy.visualization import quantity_support
import astropy.units as u
import matplotlib.pyplot as plt

quantity_support()
x = 0.5 * u.m

fig, ax = plt.subplots()
ax.axhline(x)
plt.show()

astropy-bot · 2017-12-27T20:21:23Z

Hi there @dstansby 👋 - thanks for the pull request! I'm just a friendly 🤖 that checks for issues related to the changelog and making sure that this pull request is milestoned and labeled correctly. This is mainly intended for the maintainers, so if you are not a maintainer you can ignore this, and a maintainer will let you know if any action is required on your part 😃.

I noticed the following issue with this pull request:

Changelog entry section (v3.1) inconsistent with milestone (v2.0.6)

Would it be possible to fix this? Thanks!

If there are any issues with this message, please report them here.

mhvk

Nice simple addition and much more works! Wonderful! I do think I would count this not working as a bug, and thus think we should back-port it. Could you add an entry in CHANGES.rst in the 2.0.4 section?

Also, could you add a simple test to visualization/tests/test_units.py?

mhvk · 2017-12-31T21:50:56Z

+
+    with quantity_support():
+        plt.plot([1, 2, 3] * u.m, [3, 4, 5] * u.kg)
+        plt.axhline(10 * u.cm)


A horizontal line would have to use the units of the vertical axis, no? So, maybe 10 * u.g?

Woops, that's right. I've tried that, and it throws a different error. I think there's going to need to be a bit more fixing on the Matplotlib side to use axhline actually. I'll let you know when I've had a chance to make it work properley!

Okay, with matplotlib/matplotlib#10143 and this PR axvline and axhline work properly with units. Shall I wait until the Matplotlib one is merged, and then come back to this PR?

mhvk · 2017-12-31T21:51:56Z

I think I found the reason for the test failures. When you fix it, could you also squash the commits? (I usually do this by rebasing, but if you don't know how to do it, don't worry about it.)

mhvk · 2018-01-01T16:01:36Z

@dstansby - great that you found where the matplotlib problem is (I'm not sure about the solution, though; it would seem you'd need another transformation -- I don't think you can count on multiplying with units to necessarily work in other unit systems).

For this PR, is there another matplotlib command that is enabled by your change? If so, I think we should add another test that uses that and mark the present test as xfail - with a note that this is a matplotlib bug being fixed. If you don't know about another test, then we probably should wait until the fix in matplotlib is in, and add a version-dependent xfail

dstansby · 2018-01-08T22:23:08Z

🎉 I think that errobar plotting works with this PR - I'll try and put in a test

mhvk · 2018-01-09T14:56:08Z

Would be wonderful if errorbar worked, but it looks like the X coordinate now gets messed up. Strange.

dstansby · 2018-01-09T18:10:03Z

Bah as far as I can see it's all working locally (maybe because I'm running the master branch of Matplotlib and not a release?), I'll try and have a fresh look at it soon.

dstansby · 2018-01-23T22:00:17Z

With this PR, and matplotlib/matplotlib#10206, which has just gone into Matplotlib and will be in version 2.2, axhline works properly, so I'll wait until Matplotlib 2.2 is released to update this PR. (Hopefully then lots of things can be plotted using units!)

mhvk · 2018-01-23T22:15:48Z

OK, makes sense. It will be nice if units work even more automatically!

bsipocz · 2018-01-23T23:39:14Z

@dstansby @mhvk - Since this won't be merged any time soon, I'm moving the milestone to 3.1 so we can avoid updating it in every few month. However when this ready to go, please milestone it back to the next 2.0.x milestone, to ensure it gets backported to LTS.

dstansby · 2018-04-06T13:22:25Z

Okay, this change requires Matplotlib version 2.2 or later, but I guess that's better than nothing. In theory it's all good to go now.

mhvk

@dstansby - this looks good to me, but I don't understand the many circleci failures. It seems there is something wrong with it that has little to do with your PR. @astrofrog or @bsipocz - do you have a better idea?

Anyway, approved as a PR.

dstansby · 2018-04-09T20:51:01Z

Odd, circleci doesn't even seem to be pulling the branch properly. I'll try closing and re-opening to power cycle CI.

dstansby · 2018-04-09T20:55:34Z

🤦‍♂️ I just force pushed this from an old local copy. Will update it back to the version that was approved when I get to my other computer tomorrow...

dstansby · 2018-04-18T09:57:03Z

RIGHT sorry for the faff, this is definitely now ready. pinging @astrofrog @mhvk

mhvk · 2018-04-18T16:50:55Z

Looks all OK to me. @bsipocz - at some point, you moved the milestone, but is there any reason not to backport to astropy 2.0.x? Matplotlib 2.2 still supports python2.

bsipocz · 2018-04-18T17:44:57Z

It looks it was just a placeholder to avoid unnecessary remilestoning before every bugfix. Moved it back to 2.0.x. (As a consequence, I'm happy to deal with moving the changelog to it's place if you after this is merged).

bsipocz · 2018-04-20T15:29:08Z

I'm merging as it got the all OK from @mhvk and I'll fix the milestone/changelog during release as I said above.

Thank you @dstansby!

Allow lists of quantities to be converted in Matplotlib unit converter

This reverts commit 4d9430c, reversing changes made to e96db68.

This was referenced Dec 27, 2017

Make errobars work with astropy units matplotlib/matplotlib#10118

Closed

Better deal with Numpy array subclasses in methods other than plot and scatter matplotlib/matplotlib#5896

Closed

astrofrog self-requested a review December 27, 2017 21:06

adrn requested a review from mhvk December 28, 2017 15:53

mhvk added Affects-release Bug visualization labels Dec 29, 2017

mhvk added this to the v2.0.4 milestone Dec 29, 2017

mhvk reviewed Dec 29, 2017

View reviewed changes

mhvk reviewed Dec 31, 2017

View reviewed changes

dstansby mentioned this pull request Jan 1, 2018

Correctly handle units in ax{v,h}line matplotlib/matplotlib#10143

Closed

bsipocz modified the milestones: v2.0.4, v3.1 Jan 23, 2018

dstansby mentioned this pull request Feb 2, 2018

Error when using quantities when plotting errorbars matplotlib/matplotlib#8871

Closed

mhvk approved these changes Apr 9, 2018

View reviewed changes

dstansby closed this Apr 9, 2018

dstansby reopened this Apr 9, 2018

Allow lists of quantities to be converted in Matplotlib unit converter

efaa3b3

dstansby added 2 commits April 17, 2018 16:05

Update changelog

ab6edef

Skip new test for old Matplotlib version

75a8360

bsipocz modified the milestones: v3.1, v2.0.6 Apr 18, 2018

bsipocz merged commit 4d9430c into astropy:master Apr 20, 2018

bsipocz added a commit that referenced this pull request Apr 20, 2018

Merge pull request #7037 from dstansby/list-conversion

e5f0476

Allow lists of quantities to be converted in Matplotlib unit converter

bsipocz added a commit that referenced this pull request Apr 20, 2018

Merge pull request #7037 from dstansby/list-conversion

1bb3de1

Allow lists of quantities to be converted in Matplotlib unit converter

dstansby deleted the list-conversion branch April 20, 2018 22:22

bsipocz added a commit that referenced this pull request Apr 23, 2018

Merge pull request #7037 from dstansby/list-conversion

069a597

Allow lists of quantities to be converted in Matplotlib unit converter

Cadair added a commit to Cadair/astropy that referenced this pull request May 2, 2018

Revert "Merge pull request astropy#7037 from dstansby/list-conversion"

3e6ea36

This reverts commit 4d9430c, reversing changes made to e96db68.

ritiek mentioned this pull request May 20, 2018

plot_langmuir_analysis.py example fails with astropy v3.0.2 PlasmaPy/PlasmaPy#467

Closed

pllim mentioned this pull request May 29, 2018

Commit #efaa3b3 breaks rendering of legends #7504

Closed

dstansby mentioned this pull request Sep 25, 2018

FIX: datetime64 now recognized if in a list matplotlib/matplotlib#12277

Merged

2 tasks done

Conversation

dstansby commented Dec 27, 2017

Uh oh!

astropy-bot Bot commented Dec 27, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mhvk left a comment

Choose a reason for hiding this comment

Uh oh!

mhvk Dec 31, 2017

Choose a reason for hiding this comment

Uh oh!

dstansby Jan 1, 2018

Choose a reason for hiding this comment

Uh oh!

dstansby Jan 1, 2018

Choose a reason for hiding this comment

Uh oh!

mhvk commented Dec 31, 2017

Uh oh!

mhvk commented Jan 1, 2018

Uh oh!

dstansby commented Jan 8, 2018

Uh oh!

mhvk commented Jan 9, 2018

Uh oh!

dstansby commented Jan 9, 2018

Uh oh!

dstansby commented Jan 23, 2018

Uh oh!

mhvk commented Jan 23, 2018

Uh oh!

bsipocz commented Jan 23, 2018

Uh oh!

dstansby commented Apr 6, 2018

Uh oh!

mhvk left a comment

Choose a reason for hiding this comment

Uh oh!

dstansby commented Apr 9, 2018

Uh oh!

dstansby commented Apr 9, 2018

Uh oh!

dstansby commented Apr 18, 2018

Uh oh!

mhvk commented Apr 18, 2018

Uh oh!

bsipocz commented Apr 18, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bsipocz commented Apr 20, 2018

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

astropy-bot Bot commented Dec 27, 2017 •

edited

Loading

bsipocz commented Apr 18, 2018 •

edited

Loading