/matplotlib

Fix for issue 5575 along with testing #6091

Merged
merged 1 commit into from Mar 13, 2016

Conversation

Reviewers
No reviews
Assignees
No one assigned
Labels
date handling
Projects
None yet
Milestone
  v1.5.2
4 participants
@KyleBsingh @tacaswell @QuLogic @mdboom
@KyleBsingh
Contributor

KyleBsingh commented Mar 2, 2016

This is a pull request to resolve issue #5575.

Tracing calls made in plot_date() I realized it was ignoring timezones as a result of the axis being set while plotting, then trying to change the time zone.

In line 1427 of axis.py we have converter = munits.registry.get_converter(data). Unfortunately for setting the timezone the converter will be the same as when it was plotted originally. Thus in the following lines 1433-1435 we will not be able to set_units.

default = self.converter.default_units(data, self)
if default is not None and self.units is None:
    self.set_units(default)

My solution to this was to make sure that instead of the following in _axes.py's plot_data:

if xdate:
    self.xaxis_date(tz)
if ydate:
    self.yaxis_date(tz)

ret = self.plot(x, y, fmt, **kwargs)

we have:

ret = self.plot(x, y, fmt, **kwargs)
if xdate:
    self.xaxis_date(tz)
if ydate:
    self.yaxis_date(tz)

This ensures the plot has the correct timezone, and it also plots the correct data onto the plot.
I have added test cases to /lib/matplotlib/tests/test_axes which has tests for the same and different timezones in the x axis, y axis, and both the x and y axis

mdboom added the needs_review label Mar 2, 2016

tacaswell added this to the 1.5.2 (Critical bug fix release) milestone Mar 2, 2016

@tacaswell
Owner

tacaswell commented Mar 2, 2016

Can you add only png tests for this (and re-base the pdf/svg tests out of existence)? The reason is to keep the size of the repo down and to keep the tests as fast as possible. There is nothing backend dependent about this so testing it only on Agg should be enough.

Other wise 👍

It is great when problems have simple fixes!

Can you also put the explanation of why this fixed the problem in the commit message?
@KyleBsingh
Contributor

KyleBsingh commented Mar 2, 2016

Done. Let me know if I missed anything in the pull request/commit.
@tacaswell
Owner

tacaswell commented Mar 2, 2016

Can you rebase this to only one commit? Currently the svg/pdf are still in the history which means they are (forever) in the repository.

KyleBsingh closed this Mar 2, 2016

mdboom removed the needs_review label Mar 2, 2016

@tacaswell
Owner

tacaswell commented Mar 2, 2016

If you just keep pu

tacaswell reopened this Mar 2, 2016

tacaswell added the needs_review label Mar 2, 2016

@tacaswell
Owner

tacaswell commented Mar 2, 2016

If you just keep (force) pushing to this branch github will 'do the right thing'. PRs track branches, not commits.

@QuLogic QuLogic commented on an outdated diff Mar 2, 2016

View full outdated diff
lib/matplotlib/tests/test_axes.py
@@ -4335,6 +4337,50 @@ def test_pandas_indexing_hist():
fig, axes = plt.subplots()
axes.hist(ser_2)
+@image_comparison(baseline_images=['date_timezone_x'],
+ extensions=['png'])
+def test_date_timezone_x():
+ #Tests issue 5575
+ time_index = [pytz.timezone('Canada/Eastern').localize(datetime.datetime(year=2016, month=2, day=22, hour=x)) for x in range(3)]
@QuLogic

QuLogic Mar 2, 2016

Member

This line is too long; it should be wrapped at 79 characters.

@QuLogic QuLogic commented on an outdated diff Mar 2, 2016

View full outdated diff
lib/matplotlib/tests/test_axes.py
@@ -4335,6 +4337,50 @@ def test_pandas_indexing_hist():
fig, axes = plt.subplots()
axes.hist(ser_2)
+@image_comparison(baseline_images=['date_timezone_x'],
+ extensions=['png'])
+def test_date_timezone_x():
+ #Tests issue 5575
+ time_index = [pytz.timezone('Canada/Eastern').localize(datetime.datetime(year=2016, month=2, day=22, hour=x)) for x in range(3)]
+
+ #Same Timezone
+ fig = plt.figure(figsize=(20,12))
+ plt.subplot(2, 1, 1)
+ plt.plot_date(time_index, [3]*3, tz='Canada/Eastern')
+
+ #Different Timezone
+ plt.subplot(2, 1, 2)
+ plt.plot_date(time_index, [3]*3, tz='UTC')
+
@QuLogic

QuLogic Mar 2, 2016

Member

Please use two empty lines between functions (this file is not yet checked against PEP8.) You only need to add them around the functions you've added.

@QuLogic QuLogic commented on an outdated diff Mar 2, 2016

View full outdated diff
lib/matplotlib/tests/test_axes.py
@@ -4335,6 +4337,50 @@ def test_pandas_indexing_hist():
fig, axes = plt.subplots()
axes.hist(ser_2)
+@image_comparison(baseline_images=['date_timezone_x'],
+ extensions=['png'])
+def test_date_timezone_x():
+ #Tests issue 5575
@QuLogic

QuLogic Mar 2, 2016

Member

Missing space after # (and in all other functions.)

@QuLogic QuLogic commented on an outdated diff Mar 2, 2016

View full outdated diff
lib/matplotlib/tests/test_axes.py
@@ -4335,6 +4337,50 @@ def test_pandas_indexing_hist():
fig, axes = plt.subplots()
axes.hist(ser_2)
+@image_comparison(baseline_images=['date_timezone_x'],
+ extensions=['png'])
+def test_date_timezone_x():
+ #Tests issue 5575
+ time_index = [pytz.timezone('Canada/Eastern').localize(datetime.datetime(year=2016, month=2, day=22, hour=x)) for x in range(3)]
+
+ #Same Timezone
+ fig = plt.figure(figsize=(20,12))
@QuLogic

QuLogic Mar 2, 2016

Member

Missing space after , (and in all other functions.)

@QuLogic QuLogic commented on an outdated diff Mar 2, 2016

View full outdated diff
lib/matplotlib/tests/test_axes.py
@@ -4335,6 +4337,50 @@ def test_pandas_indexing_hist():
fig, axes = plt.subplots()
axes.hist(ser_2)
+@image_comparison(baseline_images=['date_timezone_x'],
+ extensions=['png'])
+def test_date_timezone_x():
+ #Tests issue 5575
+ time_index = [pytz.timezone('Canada/Eastern').localize(datetime.datetime(year=2016, month=2, day=22, hour=x)) for x in range(3)]
+
+ #Same Timezone
+ fig = plt.figure(figsize=(20,12))
+ plt.subplot(2, 1, 1)
+ plt.plot_date(time_index, [3]*3, tz='Canada/Eastern')
@QuLogic

QuLogic Mar 2, 2016

Member

Missing spaces around * (and in all other functions.)
@KyleBsingh KyleBsingh This commit should solve issue #5575 by ensuring that plot_date first… 
… updates the units with the right timezone, before actually plotting the dates. Includes tests to ensure consistency and correctness

All checks have passed

2 successful checks
continuous-integration/appveyor/pr — AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr — The Travis CI build passed
Details
e47bc04
@KyleBsingh
Contributor

KyleBsingh commented Mar 2, 2016

Sorry about earlier, I mis-clicked "Close and comment" instead of "comment". I have rebased and made the pep8 changes that I admittedly forgot to do. Thank you

@tacaswell tacaswell merged commit e47bc04 into matplotlib:master Mar 13, 2016
2 checks passed

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@tacaswell tacaswell added a commit that referenced this pull request Mar 13, 2016

@tacaswell tacaswell Merge pull request #6091 from KyleBsingh/master 
FIX: plot_date ignores timezone

Closes #5575

Conflicts:
	lib/matplotlib/tests/test_axes.py

	   multiple PRs added tests at same location in file
	   keep all.
 ee1a98c

tacaswell removed the needs_review label Mar 13, 2016

@tacaswell tacaswell added a commit that referenced this pull request Mar 13, 2016

@tacaswell tacaswell Merge pull request #6091 from KyleBsingh/master 
FIX: plot_date ignores timezone

Closes #5575

Conflicts:
	lib/matplotlib/tests/test_axes.py

	   multiple PRs added tests at same location in file
	   keep all.
 44afc73
@tacaswell
Owner

tacaswell commented Mar 13, 2016

merged locally to resolve minor conflict ee1a98c

backported to v1.5.x as 44afc73

QuLogic added the date handling label Mar 13, 2016

@tacaswell tacaswell added a commit to tacaswell/matplotlib that referenced this pull request May 22, 2016

@tacaswell tacaswell Merge pull request #6091 from KyleBsingh/master 
FIX: plot_date ignores timezone

Closes #5575

Conflicts:
	lib/matplotlib/tests/test_axes.py

	   multiple PRs added tests at same location in file
	   keep all.
 7d3a894

dstansby referenced this pull request Feb 14, 2017

Open

plot_date() ignores timezone in matplotlib version 2.0.0 #8072

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment