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

datetime plotting broken on master #17867

Closed
dstansby opened this issue Jul 9, 2020 · 7 comments · Fixed by #17869
Closed

datetime plotting broken on master #17867

dstansby opened this issue Jul 9, 2020 · 7 comments · Fixed by #17869

Comments

@dstansby
Copy link
Member

dstansby commented Jul 9, 2020

Bug report

On master branch, plotting datetimes appears to be broken. This error bisects to a793bde in #17022

Code for reproduction

import matplotlib.pyplot as plt
from datetime import datetime

fig, ax = plt.subplots()
ax.plot(datetime.now(), 1)
plt.show()

Actual outcome

  File "test.py", line 5, in <module>
    ax.plot(datetime.now(), 1)
  File "/Users/dstansby/github/matplotlib/lib/matplotlib/axes/_axes.py", line 1745, in plot
    self.add_line(line)
  File "/Users/dstansby/github/matplotlib/lib/matplotlib/axes/_base.py", line 1977, in add_line
    self._update_line_limits(line)
  File "/Users/dstansby/github/matplotlib/lib/matplotlib/axes/_base.py", line 1999, in _update_line_limits
    path = line.get_path()
  File "/Users/dstansby/github/matplotlib/lib/matplotlib/lines.py", line 1011, in get_path
    self.recache()
  File "/Users/dstansby/github/matplotlib/lib/matplotlib/lines.py", line 653, in recache
    x = _to_unmasked_float_array(xconv).ravel()
  File "/Users/dstansby/github/matplotlib/lib/matplotlib/cbook/__init__.py", line 1298, in _to_unmasked_float_array
    return np.asarray(x, float)
  File "/Users/dstansby/miniconda3/envs/dev/lib/python3.8/site-packages/numpy/core/_asarray.py", line 83, in asarray
    return array(a, dtype, copy=False, order=order)
TypeError: float() argument must be a string or a number, not 'datetime.datetime'

Expected outcome

Matplotlib version

  • Operating system:
  • Matplotlib version:
  • Matplotlib backend (print(matplotlib.get_backend())):
  • Python version:
  • Jupyter version (if applicable):
  • Other libraries:
@dstansby
Copy link
Member Author

dstansby commented Jul 9, 2020

The issue seems to be that datetime converters are no longer being registered. If I print maptlotlib.untis.registry before plotting, it does not contain any datetime converters.

@dstansby
Copy link
Member Author

dstansby commented Jul 9, 2020

My maptlotlibrc is

backend : qt5agg
figure.figsize : 10, 8

So I think the issue is no date.converter entry means that the converters are never being registered. As an aside, does the test suite run the tests with an empty matplotlibrc at all? I'm guessing this error was never caught because the tests are always run with a fully populated matplotlibrc.

@jklymak
Copy link
Member

jklymak commented Jul 9, 2020

So at first I could not reproduce this error.

git checkout -v 3.2.1
pip install -e .
git checkout placeholder
python testDates.py

I get many rcParam errors and deprecations. If I do

git checkout placeholder
pip install -e .
python testDates.py

then I can reproduce.

To me, the underlying issue is that the matplotlibrc has to be setup properly via a pip install whereas before it just was. I believe that the change that made this an issue is #15029. From a developer point of view, its a substantial regression to have to pip install for rcParam changes. ping @anntzer about that.

I'll try and figure out how to get the rcParam set up properly here. Obviously I did something wrong in #17022.

@anntzer
Copy link
Contributor

anntzer commented Jul 9, 2020

It doesn't seem too hard to add some logic when constructing rcParamsDefault so that if it detects it's in an editable install, then it doesn't use matplotlib/mpl-data/matplotlibrc but rather matplotlibrc.template (still with the special, comment-removing loader).

(PS: I need to take a bit of time off devel because of work, so I won't implement the fix right now. As usual, if that turns out to be too bad of a blocker, feel free to revert #15029 and I'll redo it in a better way in the future.)

@jklymak
Copy link
Member

jklymak commented Jul 9, 2020

OK, but I'm confused about the pathway here. I thought the defaults were all validated when the rcParams are set up. Is that not the case?

@dstansby
Copy link
Member Author

dstansby commented Jul 9, 2020

A bit of debug, the rcParam validation is being hit (so I don't think this is #15029's fault), but sys.modules.get("matplotlib.dates") is returning None for me in the following block, which means no converter is registered.

def _validate_date_converter(s):
    s = validate_string(s)
    mdates = sys.modules.get("matplotlib.dates")
    if mdates:
        mdates._rcParam_helper.set_converter(s)

@jklymak
Copy link
Member

jklymak commented Jul 9, 2020

OK, well see the fix. Still not 100% sure this fixes all the ways the rcParams can be set up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants