FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Make set_yscale("log") consistent with semilogy() by dstansby · Pull Request #8537 · matplotlib/matplotlib · GitHub

Repository navigation

Make set_yscale("log") consistent with semilogy() - #8537

Closed
dstansby wants to merge 2 commits into
matplotlib:masterfrom
dstansby:fix-logclip
Closed

dstansby wants to merge 2 commits into
matplotlib:masterfrom
dstansby:fix-logclip

Conversation

Copy link
Copy Markdown
Member

This is an update of #8056, which fixes #8045 - I have now added the new test image, which should make the tests pass. Thanks to @jcalbert for the original fix.

emmyzero and others added 2 commits April 25, 2017 19:51
 clipping behavior as plt.semilogy(), plt.semilogx() plt.loglog()
when handing non-positive data.
dstansby added this to the 2.0.1 (next bug fix release) milestone Apr 26, 2017

phobson commented Apr 26, 2017

Copy link
Copy Markdown
Member

If you remove the re-generated image, does it pass? (I can't spot the differences in the github UI)

          ImageComparisonFailure: images not close (RMS 0.044):
          	result_images/test_axes/log_scales_svg.png
          	result_images/test_axes/log_scales-expected_svg.png

Copy link
Copy Markdown
Member Author

It seems to be the svg image (not the png one, which I updated) which is failing.

It's not failing locally on my OSX machine, and isn't failing on the OSX build either, so there must be a difference between linux and OSX when it comes to svg.

phobson commented Apr 28, 2017

Copy link
Copy Markdown
Member

@dstansby maybe you need regenerate the SVG too.

Copy link
Copy Markdown
Member Author

It's not failing on my machine, so I have no way to regenerate it! I presume if a linux box regenerated it the OSX build would fail?

QuLogic modified the milestones: 2.0.1 (next bug fix release), 2.0.2 (next bug fix release) May 3, 2017

Copy link
Copy Markdown
Member Author

(note to myself to restart travis when #8672 gets merged)

anntzer commented May 27, 2017

Copy link
Copy Markdown
Contributor

@dstansby done for you.

dstansby commented Jun 7, 2017

Copy link
Copy Markdown
Member Author

Restarting travis didn't move the build to 14.04, so manually cycling the PR.

dstansby closed this Jun 7, 2017
dstansby reopened this Jun 7, 2017

dstansby commented Jun 8, 2017

Copy link
Copy Markdown
Member Author

Huh, still no luck on travis, will try and see what happens on a local linux box at some point.

Copy link
Copy Markdown
Member

Can you check the history of why this was changed to clip? I have a vague memory that there was a long discussion at the time...

Copy link
Copy Markdown
Member Author

It was originally changed to fix #163 (comment) - changing it back will break that problem, so I adding the code in that comment as a test would be a good idea.

Copy link
Copy Markdown
Member

I do not see what changed about the test image...

efiring commented Jun 23, 2017

Copy link
Copy Markdown
Member

I think the basic idea of this--essentially reverting #2147 and thereby restoring problematic errorbar behavior (#163)--will have to be noted, and we will have to be prepared for someone to raise the issue again. Overall I think it is an improvement, though.

I think that the errorbar problem could be fixed in the errorbar code in this way: for any errorbar covering a range from negative to positive values, insert a point in the Line2D at 1e-300. It would have no effect with a linear scale, and with a log scale it would allow a line to be plotted after the negative end-point is masked out. Worth the trouble? I'm not sure.

tacaswell modified the milestones: 2.1 (next point release), 2.0.3 (next bug fix release) Jun 25, 2017
tacaswell added the API: changes Changes to the public API, typically requiring deprecation. label Jun 25, 2017

Copy link
Copy Markdown
Member

I am in favor of:

dstansby closed this Jul 3, 2017
dstansby deleted the fix-logclip branch July 3, 2017 20:34
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

API: changes Changes to the public API, typically requiring deprecation. status: duplicate status: needs revision

Projects

None yet

Development

Successfully merging this pull request may close these issues.

setting yscale to log, after drawing a plot with values equal to zero, results in incorrect handling of zero values

7 participants


Back | FazBrowse Home | New Git URL