FIX: clean up unit conversion unpacking of data, particularly for dates and pandas series by jklymak · Pull Request #11664 · matplotlib/matplotlib · GitHub
Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions lib/matplotlib/axes/_axes.py
11 changes: 9 additions & 2 deletions lib/matplotlib/axis.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

import datetime
import logging
import numbers
import warnings

import numpy as np

Expand Down Expand Up @@ -1477,9 +1479,14 @@ def have_units(self):
return self.converter is not None or self.units is not None

def convert_units(self, x):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to be converting anything numlike, which includes things like pint.Quantity, into an array/masked array before calling convert. That seems like it would break pint, though I will note that our unit tests seem to be passing.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great, I’ll check vs pint when I look at the other comments above. Thanks for the reminder...

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I guess I still don't understand how all of this works. 😉 So is_numlike only returns true for things that are basic numbers (or contain them) and don't need converting. I think I'm ok with this code then. Tests passing is always a good thing too. 😄

# If x is already a number, doesn't need converting
# If x is already a number, doesn't need converting, but
# some iterables need to be massaged a bit...
if munits.ConversionInterface.is_numlike(x):
return x
if isinstance(x, list) or isinstance(x, numbers.Number):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the rationale for returning a list as-is instead of running it through np.asarray? Just that the latter is unnecessary? Or are there cases where the conversion would be an error?

return x
if issubclass(type(x), np.ma.MaskedArray):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this using issubclass(type(x)... instead of isinstance?

return np.ma.asarray(x)
return np.asarray(x)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the point of this whole addition above to short-circuit some of what would otherwise happen in get_converter? I'm not opposed, just confused.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it seems worse, because calling np.asarray can drop information in some cases, making it unavailable in the call to convert below. Unless I'm missing something...


if self.converter is None:
self.converter = munits.registry.get_converter(x)
Expand Down
5 changes: 5 additions & 0 deletions lib/matplotlib/dates.py
Original file line number Diff line number Diff line change
Expand Up @@ -419,6 +419,11 @@ def date2num(d):
return _dt64_to_ordinalf(d)
if not d.size:
return d
if hasattr(d, 'dtype'):
if ((isinstance(d, np.ndarray) and
np.issubdtype(d.dtype, np.datetime64)) or
isinstance(d, np.datetime64)):
return _dt64_to_ordinalf(d)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What case is this whole block covering? Again, I'm confused.

return _to_ordinalf_np_vectorized(d)


Expand Down
17 changes: 4 additions & 13 deletions lib/matplotlib/testing/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -91,17 +91,8 @@ def pd():
pd = pytest.importorskip('pandas')
try:
from pandas.plotting import (
register_matplotlib_converters as register)
deregister_matplotlib_converters as deregister)
deregister()
except ImportError:
from pandas.tseries.converter import register
register()
try:
yield pd
finally:
try:
from pandas.plotting import (
deregister_matplotlib_converters as deregister)
except ImportError:
pass
else:
deregister()
pass
return pd
12 changes: 12 additions & 0 deletions lib/matplotlib/tests/test_axes.py
Original file line number Diff line number Diff line change
Expand Up @@ -5422,6 +5422,18 @@ def test_pandas_bar_align_center(pd):
fig.canvas.draw()


def test_pandas_data_unpack(pd):
# smoke test for all these different calling methods.
dates = [datetime.datetime(2018, 7, i) for i in range(1, 5)]
values = np.cumsum(np.random.rand(len(dates)))
df = pd.DataFrame({"dates": dates, "values": values})
plt.plot(df["dates"].values, df["values"])
plt.scatter(df["dates"], df["values"])
plt.plot("dates", "values", data=df)
plt.scatter(x="dates", y="values", data=df)
plt.draw()


def test_axis_set_tick_params_labelsize_labelcolor():
# Tests fix for issue 4346
axis_1 = plt.subplot()
Expand Down
10 changes: 10 additions & 0 deletions lib/matplotlib/tests/test_dates.py