BUG: fixes #12405 by eliding values index by NaT in MPLPlot._get_xticks #14540

tsdlovell · 2016-10-30T14:16:28Z

closes BUG: plotting with DatetimeIndex containing NaT #12405
tests added / passed
passes git diff upstream/master | flake8 --diff
whatsnew entry

Proposed solution: elide rows where x value would be NaT in MPLPlot._get_xticks (which seems like a minor misnomer: its getting x values, not what will ultimately be ticks).

Reordering occurs in MPLPlot._get_xticks and it has a block that is conditional on use_index and is_datetype, so seems like a reasonable place to elide the values. Mutation of self.data already occurs there (reordering) and a read through suggests nothing has saved info related to self.data's shape before this point.

tsdlovell · 2016-10-30T14:20:57Z

Will add a test in test_datetimelike.py but it will probably just repeat the sorting and eliding that occurs in the use_index/is_datetype branch of _get_xticks. Alternatively, could check that all the xvalues are within the nan{min,max} bounds of the original index.

codecov-io · 2016-10-30T14:58:40Z

Codecov Report

Merging #14540 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master   #14540      +/-   ##
==========================================
+ Coverage   90.41%   90.41%   +<.01%     
==========================================
  Files         161      161              
  Lines       50997    50999       +2     
==========================================
+ Hits        46107    46109       +2     
  Misses       4890     4890

Flag	Coverage Δ
#multiple	`88.24% <100%> (ø)`	⬆️
#single	`40.19% <50%> (ø)`	⬆️

Impacted Files	Coverage Δ
pandas/plotting/_core.py	`81.89% <100%> (+0.02%)`	⬆️

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 0f55de1...eae5c0e. Read the comment docs.

jreback · 2016-10-30T15:39:55Z

i would add some edge cases
e.g. all nans, single nan first, and single nan last

tsdlovell · 2016-10-30T22:28:42Z

@jreback , I think all NaTs should be a failure: pd.Series().plot() (data is all NaTs and we use the elide NaTs methodology) results in TypeError: Empty 'DataFrame': no numeric data to plot. Perhaps we should alert that we've elided values and that's why there is no data left.

Otherwise, all cases are the same since sort_values() results in the same series regardless of number of non-NaTs, barring all NaTs.

tsdlovell · 2016-10-30T22:31:45Z

incidentally, re @jorisvandenbossche's comment in the orignal bug report, matplotlib complains if you try to plot with NaT as an xvalue in a new plot. But once you're plotting on an axis that has already plotted with a datetime on the index, it accepts NaT values (as the min datetime value).

jreback · 2016-10-31T12:26:56Z

pandas/tests/plotting/test_datetimelike.py

+        plt.clf()
+        ax = plt.gca()
+
+        s = (tm.makeTimeSeries()


this is overly complicated, can you simplify the creation?

jreback · 2016-10-31T12:27:09Z

pandas/tests/plotting/test_datetimelike.py

+             .pipe(inject_nat_into_index_col)
+             .set_index('index', drop=True))
+        s.plot(ax=ax)
+        xdata = ax.get_lines()[0].get_xdata()


add a _check_plot_works call

jreback · 2016-10-31T12:27:47Z

pls add a release note, 0.19.1 bug fixes is fine.

jorisvandenbossche · 2016-10-31T16:34:21Z

The way matplotlib deals with missing values (for numerical data) is to leave a gap in the line. So matplotlib can actually deal with nan values. So therefore, we could also opt for this behaviour, rather than just removing those NaN values from the data before plotting.

tsdlovell · 2016-10-31T16:38:25Z

@jorisvandenbossche , we sort the index before we plot the data. Where would we insert the NaT index value?

jreback · 2016-12-16T23:54:20Z

doc/source/whatsnew/v0.19.1.txt

@@ -39,6 +39,7 @@ Bug Fixes
 - Bug in ``pd.read_csv`` for Python 2.x in which Unicode quote characters were no longer being respected (:issue:`14477`)
 - Bug in localizing an ambiguous timezone when a boolean is passed (:issue:`14402`)
 - Bug in ``TimedeltaIndex`` addition with a Datetime-like object where addition overflow in the negative direction was not being caught (:issue:`14068`, :issue:`14453`)
+- Bug in ``plot`` where ``NaT`` in ``DatetimeIndex`` results in ``Timestamp.min`` (:issue: `12405`)


can you move to 0.20.0

jreback · 2016-12-16T23:54:51Z

@tsdlovell can you rebase / update

jreback · 2017-01-21T23:16:52Z

status of this?

jreback · 2017-03-14T13:50:05Z

@tsdlovell ping!

jreback · 2017-04-18T22:38:47Z

@tsdlovell can you rebase?

tsdlovell · 2017-04-21T17:26:53Z

@jreback , I had some issues rebase'ing onto master. I ended up just applying the changes manually to the current head and creating a new branch: fix-gh12405-2.

Can I just push that to the original fork branch for this PR or will that mess something up?

jreback · 2017-04-21T22:31:27Z

yes i think u can just force push it and will work

jreback · 2017-05-13T21:38:30Z

@TomAugspurger how does this look?

jreback · 2017-05-13T21:38:40Z

doc/source/whatsnew/v0.20.0.txt

@@ -1643,6 +1643,7 @@ Plotting
 - Bug in ``DataFrame.boxplot`` where ``fontsize`` was not applied to the tick labels on both axes (:issue:`15108`)
 - Bug in the date and time converters pandas registers with matplotlib not handling multiple dimensions (:issue:`16026`)
 - Bug in ``pd.scatter_matrix()`` could accept either ``color`` or ``c``, but not both (:issue:`14855`)
+- Bug in ``plot`` where ``NaT`` in ``DatetimeIndex`` results in ``Timestamp.min`` (:issue: `12405`)


can you move to 0.20.2

TomAugspurger

Just a few changes @tsdlovell and can you move the release note to 0.20.2? Thanks!

TomAugspurger · 2017-05-15T18:05:24Z

pandas/tests/plotting/test_datetimelike.py

@@ -815,6 +815,23 @@ def test_mixed_freq_shared_ax(self):
        # self.assertEqual(ax1.lines[0].get_xydata()[0, 0],
        #                  ax2.lines[0].get_xydata()[0, 0])

+    def test_nat_handling(self):
+
+        import matplotlib.pyplot as plt  # noqa


You can remove this import and use self.plt, which comes from the base class

TomAugspurger · 2017-05-15T18:12:42Z

pandas/tests/plotting/test_datetimelike.py

+
+        import matplotlib.pyplot as plt  # noqa
+
+        fig = plt.gcf()


Is this necessary for the test to work? Our base class should take care of closing all the figures, so there shouldn't be any hanging around from a previous test.

TomAugspurger · 2017-05-15T18:13:43Z

pandas/tests/plotting/test_datetimelike.py

+        s.plot(ax=ax)
+        xdata = ax.get_lines()[0].get_xdata()
+        # plot x data is bounded by index values
+        self.assertLessEqual(s.index.min(), Series(xdata).min())


We just switched over to pytest, so this can be assert s.index.min() <= Series(xdata).min(). Same thing with the next line.

TomAugspurger · 2017-05-15T18:14:10Z

pandas/tests/plotting/test_datetimelike.py

+        # plot x data is bounded by index values
+        self.assertLessEqual(s.index.min(), Series(xdata).min())
+        self.assertLessEqual(Series(xdata).max(), s.index.max())
+        _check_plot_works(s.plot)


I think you can remove this line. It's just calling s.plot() again.

…._get_xticks TST: add test for fix of pandas-dev#12405 DOC: update whatsnew/v0.20.0.txt

tsdlovell · 2017-05-20T20:59:55Z

@TomAugspurger , let me know if there's something I still need to do.

TomAugspurger · 2017-05-20T21:59:27Z

Thanks!

…._get_xticks (pandas-dev#14540) TST: add test for fix of pandas-dev#12405 DOC: update whatsnew/v0.20.2.txt

jorisvandenbossche added Bug Datetime Datetime data dtype Visualization plotting labels Oct 30, 2016

jreback reviewed Oct 31, 2016

View reviewed changes

tsdlovell force-pushed the fix-gh12405 branch 2 times, most recently from 7963fea to 72f9d88 Compare October 31, 2016 15:44

jreback reviewed Dec 16, 2016

View reviewed changes

tsdlovell force-pushed the fix-gh12405 branch from 72f9d88 to 1048255 Compare January 28, 2017 22:25

tsdlovell force-pushed the fix-gh12405 branch from 1048255 to 1c2cad6 Compare April 22, 2017 04:35

jreback reviewed May 13, 2017

View reviewed changes

TomAugspurger requested changes May 15, 2017

View reviewed changes

BUG: fixes pandas-dev#12405 by eliding values index by NaT in MPLPlot…

eae5c0e

…._get_xticks TST: add test for fix of pandas-dev#12405 DOC: update whatsnew/v0.20.0.txt

tsdlovell force-pushed the fix-gh12405 branch from 1c2cad6 to eae5c0e Compare May 20, 2017 17:48

TomAugspurger approved these changes May 20, 2017

View reviewed changes

TomAugspurger merged commit a6fcec6 into pandas-dev:master May 20, 2017

pcluo pushed a commit to pcluo/pandas that referenced this pull request May 22, 2017

BUG: fixes pandas-dev#12405 by eliding values index by NaT in MPLPlot…

1f1e25f

…._get_xticks (pandas-dev#14540) TST: add test for fix of pandas-dev#12405 DOC: update whatsnew/v0.20.2.txt

stangirala pushed a commit to stangirala/pandas that referenced this pull request Jun 11, 2017

BUG: fixes pandas-dev#12405 by eliding values index by NaT in MPLPlot…

99855c0

…._get_xticks (pandas-dev#14540) TST: add test for fix of pandas-dev#12405 DOC: update whatsnew/v0.20.2.txt

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

BUG: fixes #12405 by eliding values index by NaT in MPLPlot._get_xticks #14540

BUG: fixes #12405 by eliding values index by NaT in MPLPlot._get_xticks #14540

tsdlovell commented Oct 30, 2016 •

edited

Loading

tsdlovell commented Oct 30, 2016

codecov-io commented Oct 30, 2016 •

edited by codecov bot

Loading

jreback commented Oct 30, 2016

tsdlovell commented Oct 30, 2016

tsdlovell commented Oct 30, 2016 •

edited

Loading

jreback Oct 31, 2016

jreback Oct 31, 2016

jreback commented Oct 31, 2016

jorisvandenbossche commented Oct 31, 2016

tsdlovell commented Oct 31, 2016

jreback Dec 16, 2016

dlovell Jan 28, 2017

jreback commented Dec 16, 2016

jreback commented Jan 21, 2017

jreback commented Mar 14, 2017

jreback commented Apr 18, 2017

tsdlovell commented Apr 21, 2017

jreback commented Apr 21, 2017

jreback commented May 13, 2017

jreback May 13, 2017

TomAugspurger left a comment

TomAugspurger May 15, 2017

TomAugspurger May 15, 2017

TomAugspurger May 15, 2017

TomAugspurger May 15, 2017

tsdlovell commented May 20, 2017

TomAugspurger commented May 20, 2017

BUG: fixes #12405 by eliding values index by NaT in MPLPlot._get_xticks #14540

BUG: fixes #12405 by eliding values index by NaT in MPLPlot._get_xticks #14540

Conversation

tsdlovell commented Oct 30, 2016 • edited Loading

tsdlovell commented Oct 30, 2016

codecov-io commented Oct 30, 2016 • edited by codecov bot Loading

Codecov Report

jreback commented Oct 30, 2016

tsdlovell commented Oct 30, 2016

tsdlovell commented Oct 30, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jreback commented Oct 31, 2016

jorisvandenbossche commented Oct 31, 2016

tsdlovell commented Oct 31, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jreback commented Dec 16, 2016

jreback commented Jan 21, 2017

jreback commented Mar 14, 2017

jreback commented Apr 18, 2017

tsdlovell commented Apr 21, 2017

jreback commented Apr 21, 2017

jreback commented May 13, 2017

Choose a reason for hiding this comment

TomAugspurger left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tsdlovell commented May 20, 2017

TomAugspurger commented May 20, 2017

tsdlovell commented Oct 30, 2016 •

edited

Loading

codecov-io commented Oct 30, 2016 •

edited by codecov bot

Loading

tsdlovell commented Oct 30, 2016 •

edited

Loading