Skip to content

Actually ignore invalid log-axis limit setting #10950

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.

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

Merged
merged 4 commits into from
Apr 23, 2018

Conversation

dstansby
Copy link
Member

@dstansby dstansby commented Apr 3, 2018

This PR makes sure the warning does what it says on the tin, which is not taking into account limits which are <= 0 on a log scaled axis.

Fixes #7733.

@dstansby dstansby added this to the v3.0 milestone Apr 3, 2018
def test_invalid_log_lims():
# Check that invalid log scale limits are ignored
fig, ax = plt.subplots()
ax.scatter(range(0, 4), range(0, 4))
Copy link
Member

Choose a reason for hiding this comment

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

Are you intentionally using data points that are not part of the valid log scale? I would have taken range(1, 3).

Copy link
Member Author

Choose a reason for hiding this comment

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

No, but I don't think it matters really, as long as the limits don't change. The test fails on the master branch, so I think it's a meaningful test.

Copy link
Member

Choose a reason for hiding this comment

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

It's definitively meaningful. I just had to put in a bit of extra thought what the actual limit before set_xlim would be. So it's slightly more complicated than need be.

Change it or leave it as you wish.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think I'll leaveit.

Choose a reason for hiding this comment

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

Would this fix #10782? In that case using negative values explicitely might make sense for a test (testing for 0 and a negative value independently, I mean).

Copy link
Member Author

Choose a reason for hiding this comment

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

No, it doesn't. I think that bug is a "scatter markers don't do autoscaling" bug.

'Attempted to set non-positive xlimits for log-scale axis; '
'invalid limits will be ignored.')
if self.get_xscale() == 'log':
if left <= 0.0:
Copy link
Member

Choose a reason for hiding this comment

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

actually 0 instead of 0.0 would be sufficient (also following occurrences). Not a merge blocker though, if you don't want to change it.

@jklymak jklymak merged commit 31e9bfe into matplotlib:master Apr 23, 2018
@dstansby dstansby deleted the log-invalid-values branch April 24, 2018 07:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants