-
-
Notifications
You must be signed in to change notification settings - Fork 8k
hist2d() is now using pcolormesh instead of pcolorfast #4625
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
Conversation
Interesting, this evidently changed the test image slightly. This might be related to #4584; it suggests there are inconsistencies in snapping or rounding among our various Artists. It looks like the problem here is only in the pdf. In this test the grid is regular, so pcolorfast was using AxesImage in the baseline image.
AxesImage is subtly different from the pcolors in that coordinates are
pixel centers versus pixel corners. Does pcolorfast take that into account?
On Sat, Jul 11, 2015 at 1:50 PM, Eric Firing notifications@github.com
wrote:
Interesting, this evidently changed the test image slightly. This might be
related to #4584 #4584;
it suggests there are inconsistencies in snapping or rounding among our
various Artists. It looks like the problem here is only in the pdf. In this
test the grid is regular, so pcolorfast was using AxesImage in the baseline
image.—
Reply to this email directly or view it on GitHub
#4625 (comment)
.
Also note that this would be an API change if accepted.
On Sat, Jul 11, 2015 at 5:02 PM, Benjamin Root ben.root@ou.edu wrote:
AxesImage is subtly different from the pcolors in that coordinates are
pixel centers versus pixel corners. Does pcolorfast take that into account?On Sat, Jul 11, 2015 at 1:50 PM, Eric Firing notifications@github.com
wrote:Interesting, this evidently changed the test image slightly. This might
be related to #4584
#4584; it suggests
there are inconsistencies in snapping or rounding among our various
Artists. It looks like the problem here is only in the pdf. In this test
the grid is regular, so pcolorfast was using AxesImage in the baseline
image.—
Reply to this email directly or view it on GitHub
#4625 (comment)
.
@WeatherGod, Yes, pcolorfast does take into account the way AxesImage works. The travis failure is coming from a very subtle shift in some boundaries in the pdf output.
@efiring @WeatherGod did you happen to remember if you were leaning towards merging or more discussion at last comment?
The test image would have to be changed. Once this is rebased and all tests pass, I would favor merging it. It would be nice to fix pcolorfast to work correctly with log scales, but that's a bigger job.
I don't see why this would be considered an API change.
I agree that it would be an API change; not a particularly disruptive one, but by definition, return types are part of the API.
Pull request in response to issue #4615