[MRG+1] plot_date() after axhline() doesn't rescale axes by Yasaman-Mah · Pull Request #8205 · matplotlib/matplotlib

Yasaman-Mah · 2017-03-06T15:52:53Z

The only reason that plotting axhline first changes behaviour of plotting datetime is that it sets ignore_existing_data_limits to false and then when plotting datetime data, default datetime data limits are taken into consideration.
Unfortunately this fix would affect y data limits too. I think a better solution would be to have two variables ignore_existing_x_limits and ignore_existing_y_limits, but I am not sure if adding more detail and making it slightly more complicated just because of axhline makes sense.

ref #7742

pull

update

phobson · 2017-03-06T15:57:08Z

    # Check that a horizontal chart with one width works
    ax.bar(0, 1, bottom=range(4), width=1, orientation='horizontal')
+
+def test_datetime_axhline_same_axes():


The pep8 checker is probably going to want 2 blank lines between functions

Looks like you'll also need to trim trailing whitespace:
https://travis-ci.org/matplotlib/matplotlib/jobs/208246492#L1683

NelleV · 2017-03-06T20:34:59Z

Next time, please add a meaningful title to the pull requests. The reference to the ticket should be in in the description of the PR (else it is non clickable).

Modified axhline() function to save state of ignoring limits and restore after plotting the line

Shang-Jia · 2017-03-07T18:48:48Z

This solution will always reset the existing_data_limits so on the following execution
plot() -> sets data limits
axhline() -> allows data limits to be set again
plot() -> sets data limits again, even if data limits are smaller than what was previously plotted
see testcase and proposed solution in #8210

Yasaman-Mah · 2017-03-08T01:15:21Z

@Shang-Jia This is a preliminary solution, I submitted this pull request to get some feedback from matplotlib team before going on with another solution.

NelleV · 2017-03-11T23:21:05Z

This looks good to me. Thanks for the patch and sorry it took so long to get feedback!

tacaswell · 2017-03-12T21:41:28Z

        trans = self.get_yaxis_transform(which='grid')
        l = mlines.Line2D([xmin, xmax], [y, y], transform=trans, **kwargs)
        self.add_line(l)
+        self.ignore_existing_data_limits = True


Can you add a symmetric change to axvline ?

I have made a similar change to axvline. Please see my comment below before approving the changes.

Yasaman-Mah · 2017-03-16T03:17:43Z

This solution has a side effect, it will result in inconsistent behaviour when setting y axis limits. For example:

`import matplotlib.pyplot as plt

fig, axs = plt.subplots(2, 1)

axs[0].axvline(1)
axs[0].plot([2, 3], [2, 3])

axs[1].plot([2, 3], [2, 3])
axs[1].axvline(1)

plt.show()`

will result in following plots:

which are not wrong but inconsistent.
However, I believe currently there is no way to avoid this unless we add one more field to axes object or make more substantial changes to how limits are set.

ibnIrshad · 2017-03-18T16:33:56Z

@Yasaman-Mah It's good you pointed that out. However I still think that this PR is a useful contribution and should be merged.

This PR fixes #7742 (which produces clearly wrong output), and although it does introduce a side effect, the side effect is not "wrong output", but only slightly inconsistent for a certain specific order of input. And as you pointed out, it would require MUCH more work to fix this inconsistency, because introducing two symmetric flags "ignore_y_limits" and "ignore_x_limits" is a significant architectural change.

Hence I would ask @NelleV @tacaswell to review this PR again. Can we go ahead with this fix?

tacaswell · 2017-03-18T16:58:09Z

What are dates (which are just input with units) behaving differently than plain numbers?

I am still not sure I understand what the self.ignore_existing_data_limits flag does and when it gets set/unset.

The code and the figure above seem out of sync with each other, but I would not call that a 'side effect'. IT looks like the bug from #7742 (order dependent limits) has just been moved to different (and probably more common) case.

tacaswell

Still not sure I understand what is going on here.

Anyone who can may clear this review.

jklymak · 2018-03-24T04:36:27Z

Can this PR be resurrected or closed?

jklymak · 2019-02-07T17:41:50Z

I'm closing for now - but feel free to re-open if someone wants to champion this PR. Thanks for the contribution.

anntzer and others added 6 commits February 4, 2017 20:03

Allow choosing logit scale in qt figure options.

f758834

Merge pull request #2 from matplotlib/master

24b21ae

pull

Merge pull request #3 from matplotlib/master

45eadc1

update

fix matplotlib#7742

7eceed5

test case for bug fix 7742

52beee2

fixed indentation

a3dbc3c

phobson reviewed Mar 6, 2017

View reviewed changes

NelleV changed the title ~~issue #7742~~ plot_date() after axhline() doesn't rescale axes Mar 6, 2017

phobson mentioned this pull request Mar 7, 2017

Fixes #7742 - axhline rescale issue #8210

Closed

ibnIrshad referenced this pull request in sughandj/matplotlib Mar 7, 2017

Fixes matplotlib#7742 - axhline rescale issue

7cf097d

Modified axhline() function to save state of ignoring limits and restore after plotting the line

ibnIrshad mentioned this pull request Mar 7, 2017

Fixes #7742 - axhline rescale issue kenmaca/matplotlib#3

Merged

Isa Hassen added 3 commits March 7, 2017 07:38

fix pep8

b37ffc5

fix merge conflict (add both tests)

e083a81

Merge branch 'matplotlib-master' into fix7742

ad03e22

NelleV changed the title ~~plot_date() after axhline() doesn't rescale axes~~ [MRG+1] plot_date() after axhline() doesn't rescale axes Mar 11, 2017

NelleV approved these changes Mar 12, 2017

View reviewed changes

tacaswell reviewed Mar 12, 2017

View reviewed changes

Yasaman-Mah added 2 commits March 15, 2017 22:33

added similar changes to axvline.

4fa59c6

resolved merge conflicts in test_axes.py.

4e516f2

fixed pep8 errors.

8c9ef18

tacaswell requested changes Mar 18, 2017

View reviewed changes

dstansby added this to the 2.1.1 (next bug fix release) milestone Aug 4, 2017

tacaswell removed this from the 2.1.1 (next bug fix release) milestone Oct 9, 2017

tacaswell added this to the 2.2 (next feature release) milestone Oct 9, 2017

jklymak modified the milestones: needs sorting, v3.0 Mar 24, 2018

jklymak added the topic: date handling label Mar 24, 2018

tacaswell modified the milestones: v3.0, v3.1 Jul 7, 2018

jklymak closed this Feb 7, 2019

Uh oh!

Conversation

Yasaman-Mah commented Mar 6, 2017 • edited by NelleV Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

phobson Mar 6, 2017

Choose a reason for hiding this comment

Uh oh!

phobson Mar 6, 2017

Choose a reason for hiding this comment

Uh oh!

NelleV commented Mar 6, 2017

Uh oh!

Shang-Jia commented Mar 7, 2017

Uh oh!

Yasaman-Mah commented Mar 8, 2017

Uh oh!

NelleV commented Mar 11, 2017

Uh oh!

tacaswell Mar 12, 2017

Choose a reason for hiding this comment

Uh oh!

Yasaman-Mah Mar 17, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Yasaman-Mah commented Mar 16, 2017

Uh oh!

ibnIrshad commented Mar 18, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tacaswell commented Mar 18, 2017

Uh oh!

tacaswell left a comment

Choose a reason for hiding this comment

Uh oh!

jklymak commented Mar 24, 2018

Uh oh!

jklymak commented Feb 7, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

Yasaman-Mah commented Mar 6, 2017 •

edited by NelleV

Loading

Yasaman-Mah Mar 17, 2017 •

edited

Loading

ibnIrshad commented Mar 18, 2017 •

edited

Loading