Skip to content

BUG: Plotting Timedelta on y-axis #16953 #17430

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 10 commits into from
Sep 6, 2017
2 changes: 1 addition & 1 deletion doc/source/whatsnew/v0.21.0.txt
Original file line number Diff line number Diff line change
Expand Up @@ -400,7 +400,7 @@ I/O
Plotting
^^^^^^^^
- Bug in plotting methods using ``secondary_y`` and ``fontsize`` not setting secondary axis font size (:issue:`12565`)

- Bug when plotting Timedelta and Datetime on y-axis (:issue:`16953`)
Copy link
Contributor

Choose a reason for hiding this comment

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

say timedelta and datetime dtypes (and use double back-ticks).


Groupby/Resample/Rolling
^^^^^^^^^^^^^^^^^^^^^^^^
Expand Down
4 changes: 3 additions & 1 deletion pandas/plotting/_core.py
Original file line number Diff line number Diff line change
Expand Up @@ -342,7 +342,9 @@ def _compute_plot_data(self):
label = 'None'
data = data.to_frame(name=label)

numeric_data = data._convert(datetime=True)._get_numeric_data()
# GH16953
data = data._convert(datetime=True, timedelta=True)
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think we need the _convert at all here. this is for object types that need to be coerced. almost all routines in pandas already assume this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The _convert is needed for a test, where a Series with numeric values and dtype object is used (tests.plotting.test_series.test_valid_object_plot).
If _convert isn't used, that test will fail in circleci.
To prevent the test from failing, _convert can either be used only on Series with dtype==object and after the to_frame (line 344), or on data in general after the Series specific part (line 346 as is now).

Copy link
Contributor

Choose a reason for hiding this comment

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

did you take out the _convert and see, on what input does it fail?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yap i did, circleci fails at tests.plotting.test_series.test_valid_object_plot, since the Series is of dtype == object (s = Series(lrange(10), dtype=object), also see comment on your last requested changes).
Also i think doing a general data._convert is more sturdy, than just calling in if the Seriesis of type object (which would be the minimal change for the test to pass).

Copy link
Contributor

Choose a reason for hiding this comment

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

ok I guess leave it in is ok. In general we don't automatically coerce object types (even if they are numbers) in other functions / places; we happen to do it in plotting for compat I think. Can you open a new issue for this to discuss (deprecating this).

numeric_data = data.select_dtypes(include=[np.number, "datetime", "timedelta"])

try:
is_empty = numeric_data.empty
Expand Down
23 changes: 23 additions & 0 deletions pandas/tests/plotting/test_frame.py
Original file line number Diff line number Diff line change
Expand Up @@ -380,6 +380,29 @@ def test_subplots_timeseries(self):
self._check_ticks_props(ax, xlabelsize=7, xrot=45,
ylabelsize=7)

def test_subplots_timeseries_y_axis(self):
# GH16953
data = {"numeric": np.array([1, 2, 5]),
"timedelta": [pd.Timedelta(10, unit="s"),
pd.Timedelta(10, unit="m"),
pd.Timedelta(10, unit="h")],
"datetime": [pd.to_datetime("2017-08-01 00:00:00"),
Copy link
Contributor

Choose a reason for hiding this comment

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

can you add a datetime with a tz as well

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure ^^

Copy link
Contributor

Choose a reason for hiding this comment

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

I think some other types, e.g. Period might fail, if you can add them in another test with xfail would be fine (Categoricals might work as they area converted to ndarrays)

pd.to_datetime("2017-08-01 02:00:00"),
pd.to_datetime("2017-08-02 00:00:00")],
"text": ["This", "should", "fail"]}
testdata = DataFrame(data)
ax_numeric = testdata.plot(y="numeric")
assert (ax_numeric.get_lines()[0].get_data()[1] ==
testdata["numeric"].values).all()
ax_timedelta = testdata.plot(y="timedelta")
assert (ax_timedelta.get_lines()[0].get_data()[1] ==
testdata["timedelta"].values).all()
ax_datetime = testdata.plot(y="datetime")
assert (ax_datetime.get_lines()[0].get_data()[1] ==
testdata["datetime"].values).all()
with pytest.raises(TypeError):
testdata.plot(y="text")

@pytest.mark.slow
def test_subplots_layout(self):
# GH 6667
Expand Down