Skip to content
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

Mollweide latitude grid #2306

Merged
merged 1 commit into from Aug 27, 2013

Conversation

lpsinger
Copy link
Contributor

The function _get_pixel_distance_along_axis does not work for geographic
axes, in which pixels that are outside of the map area cannot be
meaningfully transformed back to data coordinates. To be safe, let us
just return 0.0 unless we are on rectilinear axes.

Fixes #2299.

@@ -1657,7 +1657,7 @@ def _get_pixel_distance_along_axis(self, where, perturb):
# do things correctly, we need to use rmax instead of 1e-10 for a polar axis. But
# since we do not have that kind of information at this point, we just don't try to
# pad anything for the theta axis of a polar plot.
if self.axes.name == 'polar':
if self.axes.name != 'rectilinear':
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if this might break things for mplot3d. I see what you are trying to fix, but there has to be some other way to identify the axes that should have 0 returned for this function. Perhaps each axes type should have a function defined for themselves to return the appropriate value?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm, that would be an actual API change and would require modifications to many axes subclasses so that the gridlines would appear.

What if we change this back to if self.axes.name == 'polar', but then If the return value of this function is nan (because transform can't give meaningful answers outside of projection region), the pixel distance is assumed to be 0?

@lpsinger
Copy link
Contributor Author

@WeatherGod, if you like this, I can squash together commits 1a2a0bb and 1977776.

@mdboom
Copy link
Member

mdboom commented Aug 26, 2013

👍 from me if @WeatherGod agrees. And it should be backported to v1.3.x.

The function _get_pixel_distance_along_axis does not work for geographic
axes, in which pixels that are outside of the map area cannot be
meaningfully transformed back to data coordinates. If the return value
was nan, set to 0.
@lpsinger
Copy link
Contributor Author

Squashed all commits.

@WeatherGod
Copy link
Member

Actually, I think this is a perfectly good solution (much better than my idea).

@lpsinger
Copy link
Contributor Author

I see this Travis failure:

======================================================================
FAIL: matplotlib.tests.test_image.test_rasterize_dpi.test
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/travis/virtualenv/python3.3/lib/python3.3/site-packages/nose/case.py", line 198, in runTest
    self.test(*self.arg)
  File "/home/travis/virtualenv/python3.3/lib/python3.3/site-packages/matplotlib-1.4.x-py3.3-linux-x86_64.egg/matplotlib/testing/decorators.py", line 40, in failer
    result = f(*args, **kwargs)
  File "/home/travis/virtualenv/python3.3/lib/python3.3/site-packages/matplotlib-1.4.x-py3.3-linux-x86_64.egg/matplotlib/testing/decorators.py", line 164, in do_test
    '(RMS %(rms).3f)'%err)
matplotlib.testing.noseclasses.ImageComparisonFailure: images not close: /home/travis/build/lpsinger/tmp_test_dir/result_images/test_image/rasterize_10dpi_svg.png vs. /home/travis/build/lpsinger/tmp_test_dir/result_images/test_image/rasterize_10dpi-expected_svg.png (RMS 0.004)

but it appears to be unrelated. And I have found from recent experience that restarting the build fixes it. This seems to happen only under Python 3.

@WeatherGod
Copy link
Member

That is a weird bug, and the RMS error on it is extremely tiny. Could it possibly be a discrepency in different versions of the baseline image (or how they are loaded up)?

@lpsinger
Copy link
Contributor Author

Travis build fixed itself again. @WeatherGod, is there anything else that you want to see in this PR?

@WeatherGod
Copy link
Member

LGTM +1

WeatherGod added a commit that referenced this pull request Aug 27, 2013
@WeatherGod WeatherGod merged commit 88f8b8f into matplotlib:master Aug 27, 2013
@lpsinger lpsinger deleted the mollweide_latitude_grid branch August 27, 2013 14:55
WeatherGod added a commit that referenced this pull request Aug 27, 2013
@@ -1190,6 +1190,14 @@ def test_transparent_markers():
ax = fig.add_subplot(111)
ax.plot(data, 'D', mfc='none', markersize=100)

@image_comparison(baseline_images=['mollweide_grid'], remove_text=True)
Copy link
Member

Choose a reason for hiding this comment

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

I know its been merged so we don't need to worry about it, but for future reference we only needed to test one file format (png would be great) - this isn't a backend issue so we needn't repeat out testing.

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.

Mollweide projection no longer shows horizontal gridlines
4 participants