Skip to content

BUG: Fix the un-pickleable plot with DatetimeIndex #18486

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Dec 6, 2017
Merged

BUG: Fix the un-pickleable plot with DatetimeIndex #18486

merged 3 commits into from
Dec 6, 2017

Conversation

Licht-T
Copy link
Contributor

@Licht-T Licht-T commented Nov 25, 2017 •
edited
Loading

@jreback
Copy link
Contributor

jreback commented Nov 25, 2017

can u add a test as well

@Licht-T
Copy link
Contributor Author

Licht-T commented Nov 25, 2017 •
edited
Loading

@jreback Where should I add test? Inside pandas/tests/plotting/?

@jreback
Copy link
Contributor

jreback commented Nov 25, 2017

sure

@jreback jreback added Compat pandas objects compatability with Numpy or Python functions Visualization plotting labels Nov 25, 2017
@jreback
Copy link
Contributor

jreback commented Nov 25, 2017

0.21.1 release note as well, yes pandas/tests/test_plotting is ok (or maybe in a misc section in plotting).

@codecov
Copy link

codecov bot commented Nov 25, 2017 •
edited
Loading

Codecov Report

Merging #18486 into master will decrease coverage by 0.01%.
The diff coverage is 75%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #18486      +/-   ##
==========================================
- Coverage   91.59%   91.58%   -0.02%     
==========================================
  Files         155      153       -2     
  Lines       51255    51256       +1     
==========================================
- Hits        46948    46943       -5     
- Misses       4307     4313       +6
Flag Coverage Δ
#multiple 89.44% <75%> (ø) ⬆️
#single 40.67% <0%> (-0.11%) ⬇️
Impacted Files Coverage Δ
pandas/plotting/_timeseries.py 60.82% <75%> (+0.09%) ⬆️
pandas/io/gbq.py 25% <0%> (-58.34%) ⬇️
pandas/core/frame.py 97.81% <0%> (-0.1%) ⬇️
pandas/io/wb.py
pandas/io/data.py
pandas/util/testing.py 82.01% <0%> (+0.19%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a764663...a3d909b. Read the comment docs.

@Licht-T
Copy link
Contributor Author

Licht-T commented Nov 26, 2017

Thanks @jreback, fixed!

@jreback jreback added this to the 0.21.1 milestone Nov 26, 2017
@jreback
Copy link
Contributor

jreback commented Nov 26, 2017

lgtm. ping on green.

@Licht-T
Copy link
Contributor Author

Licht-T commented Nov 26, 2017

CI failed because of the test. I'll check.

@Licht-T
Copy link
Contributor Author

Licht-T commented Nov 26, 2017

@jreback Seems that the pickling instancemethod only works in Python 3.

@Licht-T Licht-T force-pushed the fix-datetimeindex-plot-not-pickleable branch from 87d082e to 8797d20 Compare November 26, 2017 14:57
@Licht-T
Copy link
Contributor Author

Licht-T commented Nov 26, 2017

Now added the restriction to Python 3.

jreback
jreback reviewed Nov 26, 2017
View reviewed changes
pandas/tests/plotting/test_datetimelike.py Outdated

# GH18439
if PY3:
with ensure_clean(return_filelike=True) as path:
Copy link
Contributor

Choose a reason for hiding this comment

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

could test for the round trip here, but prob overkill

jreback
jreback reviewed Nov 26, 2017
View reviewed changes
pandas/tests/plotting/test_datetimelike.py
@@ -1470,5 +1471,10 @@ def _check_plot_works(f, freq=None, series=None, *args, **kwargs):

with ensure_clean(return_filelike=True) as path:
plt.savefig(path)

# GH18439
Copy link
Contributor

Choose a reason for hiding this comment

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

why is this only PY3?

add a more informative comment, not just the issue, e.g. ensure round-trip pickle compat

Copy link
Contributor

Choose a reason for hiding this comment

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

@Licht-T saw your comment, ok then, just make a comment that we are pickling instancemethods which is supported on PY3 only (hmm thought this did work on PY2)

jreback
jreback requested changes Nov 26, 2017
View reviewed changes
pandas/tests/plotting/test_datetimelike.py
@@ -1470,5 +1471,10 @@ def _check_plot_works(f, freq=None, series=None, *args, **kwargs):

with ensure_clean(return_filelike=True) as path:
plt.savefig(path)

# GH18439
Copy link
Contributor

Choose a reason for hiding this comment

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

@Licht-T saw your comment, ok then, just make a comment that we are pickling instancemethods which is supported on PY3 only (hmm thought this did work on PY2)

@jreback
Copy link
Contributor

jreback commented Nov 27, 2017

can you update

@Licht-T
Copy link
Contributor Author

Licht-T commented Nov 27, 2017

@jreback Okay! I'll re-check the pickling issue on Python 2 and update until tomorrow.

@jreback
Copy link
Contributor

jreback commented Dec 2, 2017

can you update

@Licht-T Licht-T force-pushed the fix-datetimeindex-plot-not-pickleable branch from 8797d20 to a3d909b Compare December 5, 2017 00:20
@Licht-T
Copy link
Contributor Author

Licht-T commented Dec 5, 2017

Sorry @jreback! Now updated!

@jreback
Copy link
Contributor

jreback commented Dec 5, 2017

lgtm
ping on green

@jreback jreback merged commit 2145e89 into pandas-dev:master Dec 6, 2017
@jreback
Copy link
Contributor

jreback commented Dec 6, 2017

thanks!

TomAugspurger pushed a commit to TomAugspurger/pandas that referenced this pull request Dec 8, 2017
@Licht-T @TomAugspurger
BUG: Fix the un-pickleable plot with DatetimeIndex (pandas-dev#18486)
9291ba3
TomAugspurger pushed a commit that referenced this pull request Dec 11, 2017
@Licht-T @TomAugspurger
BUG: Fix the un-pickleable plot with DatetimeIndex (#18486)
c763ecf
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jreback jreback jreback approved these changes

Assignees
No one assigned
Labels
Compat pandas objects compatability with Numpy or Python functions Visualization plotting
Projects
None yet
Milestone
0.21.1
Development

Successfully merging this pull request may close these issues.

Can't pickle plots from dataframes with date indexes
3 participants
@Licht-T @jreback @TomAugspurger