BUG: Fix TimeFormatter behavior with fractional seconds #18552
Conversation
Codecov Report
@@ Coverage Diff @@
## master #18552 +/- ##
==========================================
- Coverage 91.63% 91.57% -0.07%
==========================================
Files 154 153 -1
Lines 51422 51366 -56
==========================================
- Hits 47123 47039 -84
- Misses 4299 4327 +28
Continue to review full report at Codecov.
|
| @@ -107,19 +107,22 @@ def __init__(self, locs): | |||
| self.locs = locs | |||
|
|
|||
| def __call__(self, x, pos=0): | |||
| fmt = '%H:%M:%S' | |||
There was a problem hiding this comment.
can you add a doc-string here
There was a problem hiding this comment.
cam you add a Parameters sections
| # time2num(datetime.time.min) | ||
| rs = self.tc(0) | ||
| xp = '00:00' | ||
| assert rs == xp |
There was a problem hiding this comment.
can you add a blank line before the comments
| @@ -112,7 +112,24 @@ def test_conversion_outofbounds_datetime(self): | |||
| assert rs == xp | |||
|
|
|||
| def test_time_formatter(self): | |||
| self.tc(90000) | |||
There was a problem hiding this comment.
add the issue number here as a comment
| xp = '23:59:59.999999' | ||
| assert rs == xp | ||
| # some other times | ||
| rs = self.tc(90000) |
There was a problem hiding this comment.
I think we need a test from the original issue, which check that the plot works
nmartensen
force-pushed
the
time_formatter
branch
3 times, most recently
from
878687a to
200528b
Compare
|
Do you know why GH still says "changes requested"? I believe all comments are addressed. |
doc/source/whatsnew/v0.21.1.txt
Outdated
| @@ -125,6 +125,7 @@ Plotting | |||
| ^^^^^^^^ | |||
|
|
|||
| - Bug in ``DataFrame.plot()`` and ``Series.plot()`` with :class:`DatetimeIndex` where a figure generated by them is not pickleable in Python 3 (:issue:`18439`) | |||
| - Bug in :class:`TimeFormatter` when trying to display fractional seconds (:issue:`18478`). | |||
|
Example code: ax = pandas.DataFrame({'A': 1}, index=[pydt.time(11, 2, 59, 899502), pydt.time(11, 3, 0, 123456)]).plot()
for l in ax.xaxis.get_majorticklabels():
l.set_rotation(45)
l.set_horizontalalignment('right')Output before: Output after: |
|
is it standard to only show the most resolved, IOW, |
|
My original motivation to change the TimeFormatter was to get rid of the :SS since I was always plotting full days. But if you prefer, we can drop that commit. I didn't discover the sub-second breakage from a real usecase, but while reading the code to figure out where I'd need to change it for the above. |
do we do a similar thing for fractional datetimes? cc @TomAugspurger @jorisvandenbossche @tacaswell |
|
The ideal way would probably be to not show the seconds if they are zero for each tick position, and show them if they are non-zero for at least one tick position. But I don't know how to implement that. |
|
For comparison some plots illustrating tick labels with datetimes. Code: fig, axes = plt.subplots(nrows=6, ncols=1, figsize=(6, 10))
for i, ut in enumerate(['H', 'min', 'S', 'ms', 'us', 'N']):
dti = pandas.date_range('11:02:59.889502', periods=2, freq='200'+ut)
pandas.DataFrame({'A': 1}, index=dti).plot(ax=axes[i])
axes[i].set_title('delta t = 200'+ut, position=(0.5, 0.6))
axes[i].text(0.5, 0.1, str(dti.max()-dti.min()), transform=axes[i].transAxes, ha='center')
plt.tight_layout()Resulting output: |
nmartensen
force-pushed
the
time_formatter
branch
from
56a8ebd to
c1ace38
Compare
|
thanks @nmartensen |
git diff upstream/master -u -- "*.py" | flake8 --diff