Skip to content

Addresses #8045 (set_yscale("log") inconsistent with semilogy()) #8056

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

Closed
wants to merge 1 commit into from

Conversation

jcalbert
Copy link
Contributor

See: #8045 "setting yscale to log, after drawing a plot with values equal to zero, results in incorrect handling of zero values "

 clipping behavior as plt.semilogy(), plt.semilogx() plt.loglog()
when handing non-positive data.
@@ -2943,7 +2943,7 @@ def set_xscale(self, value, **kwargs):
# If the scale is being set to log, clip nonposx to prevent headaches
Copy link
Contributor

Choose a reason for hiding this comment

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

update the comment too? (below as well)

@tacaswell
Copy link
Member

Came in via 7898bbb / #2147

The test failures look real (likely in the anti-alias of the line along the edge).

@jcalbert
Copy link
Contributor Author

I spent a while hunting this down until I realized that, in fact, the before- and after- images should look different. For the moment I can't figure out how the old test conspired to make clip and mask so similar.

One possibility is the way MPL chooses the axis bounds. Shown below are four plots, the Cartesian product of (clip, mask) and data which starts at x= -2 and x=0.
mpl1

compare the above output from mpl 1.5.1 to this from mpl 2.0.0:
mpl2

is is evident that mpl1 has chosen x-axis limits that mostly obscure the difference between clip and mask.

It seems that whichever behavior we want, we will need a test that more explicitly checks the distinction.

@anntzer
Copy link
Contributor

anntzer commented Apr 16, 2017

Most old tests still use the classic style, where the plots use "round" numbers as limits, so that's probably why they still passed. Updating the test images should make this good.

@dstansby
Copy link
Member

This ended up being superseeded by #8836, thanks for the PR though!

@dstansby dstansby closed this Jul 26, 2017
@jcalbert jcalbert deleted the fix-logclip branch May 3, 2019 23:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants