Skip to content

Fix pcolormesh and DatetimeIndex error #9168

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

Merged
merged 3 commits into from
Sep 22, 2017

Conversation

jklymak
Copy link
Member

@jklymak jklymak commented Sep 8, 2017

PR Summary

Fixes issue with DatetimeIndex and pcolormesh: #9167

PR Checklist

  • [X ] Code is PEP 8 compliant

@jklymak
Copy link
Member Author

jklymak commented Sep 8, 2017

Test is

import matplotlib.pyplot as plt
import numpy as np
import pandas as pd

time = pd.date_range('2000-01-01', periods=10)
depth = np.arange(20)
data = np.random.rand(20,10)

fig, ax = plt.subplots()
ax.pcolormesh(time, depth, data)
plt.show()

@jklymak
Copy link
Member Author

jklymak commented Sep 8, 2017

ping @QuLogic who made the original change. I think this is compatible w/ your change, but still lets us use DatetimeIndex....

@dstansby dstansby added the Release critical For bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions. label Sep 8, 2017
@dstansby dstansby added this to the 2.1 (next point release) milestone Sep 8, 2017
@anntzer anntzer requested a review from QuLogic September 8, 2017 18:51
@tacaswell
Copy link
Member

@jklymak We have a helper to skip or import depending on if pandas is installed. I took the liberty of putting your test code in and pushed to your branch (just in the interest of expediency).

@QuLogic
Copy link
Member

QuLogic commented Sep 18, 2017

This seems to make sense in general, but I'm just wondering why convert_?units returns a list instead of an array-like type.

@jklymak
Copy link
Member Author

jklymak commented Sep 18, 2017

@QuLogic It appears convert_?units can accept list-like objects and doesn't do any type conversion. I'm not up enough on pandas objects to know what would happen if you tried to change the type, but I think a lot of the time-based tick labelling depends on the pandas object, so I'd be loathe to change it.

@efiring
Copy link
Member

efiring commented Sep 20, 2017

I think the DatetimeIndex object (the time sequence) is not recognized (it is not in the units registry), so it is being treated as a list of Timestamp objects (which are in the registry). I haven't been able to track down exactly how this is happening. Units handling is convoluted, to say the least, and all the more so when it is split between matplotlib and pandas, which is calling back into matplotlib. In any case, the pandas DatetimeIndex object fortunately has a ravel method which has the same effect is the values attribute, yielding an ndarray of dtype 'np.datetime64[ns]'. This dtype is handled by the same registered converter as the Timestamp would be, tseries.converter.DatetimeConverter.

@tacaswell
Copy link
Member

[testing for the jupyter backport-bot that this is a likely candidate for using soon]

@meeseeksdev hello

@lumberbot-app
Copy link

lumberbot-app bot commented Sep 20, 2017

I'm Mr. Meeseek, @tacaswell, Look at meee !

@tacaswell tacaswell merged commit ffa4803 into matplotlib:master Sep 22, 2017
@tacaswell
Copy link
Member

@meeseeksdev backport to v2.1.x

lumberbot-app bot pushed a commit that referenced this pull request Sep 22, 2017
<!--Thank you so much for your PR! To help us review, fill out the form
to the best of your ability.  Please make use of the development guide at
https://matplotlib.org/devdocs/devel/index.html-->

<!--Provide a general summary of your changes in the title above, for
example "Raises ValueError on Non-Numeric Input to set_xlim".  Please avoid
non-descriptive titles such as "Addresses issue  8576".-->

<!--If you are able to do so, please do not create the
PR out of master, but out of a separate branch.  See
https://matplotlib.org/devel/gitwash/development_workflow.html for
instructions.-->

   PR Summary

Fixes issue with `DatetimeIndex` and `pcolormesh`: #9167

<!--Please provide at least 1-2 sentences describing the pull request in
detail.  Why is this change required?  What problem does it solve?-->

<!--If it fixes an open issue, please link to the issue here.-->

   PR Checklist

- [X ] Code is PEP 8 compliant

<!--We understand that PRs can sometimes be overwhelming, especially as the
reviews start coming in.  Please let us know if the reviews are unclear or the
recommended next step seems overly demanding , or if you would like help in
addressing a reviewer's comments.  And please ping us if you've been waiting
too long to hear back on your PR.-->
tacaswell added a commit that referenced this pull request Sep 22, 2017
@jklymak jklymak deleted the fixpcolormeshdatetime branch September 24, 2017 17:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Release critical For bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants