Skip to content
This repository has been archived by the owner on Jun 16, 2018. It is now read-only.

Compute tick angles by advancing a small amount in axes coordinates #46

Merged
merged 2 commits into from
Jun 19, 2014

Conversation

lpsinger
Copy link
Contributor

This handles cases when a plot extends over a small number of pixels in the WCS projection, but a large distance or number of pixels in display coordinates. Addresses #45.

Here is a modified test script. The ticks are red and extra-long to make them stand out better.

#!/usr/bin/env python
from matplotlib import pyplot as plt
from wcsaxes import WCSAxes
import astropy.wcs

wcs = astropy.wcs.WCS()
wcs.wcs.ctype = ['RA---TAN', 'DEC--TAN']
wcs.wcs.crval = [90, 70]
wcs.wcs.cdelt = [16, 16]
wcs.wcs.crpix = [1, 1]
wcs.wcs.radesys = 'ICRS'
wcs.wcs.equinox = 2000.0
fig = plt.figure(figsize=(3, 3), dpi=72)
ax = WCSAxes(fig, [0.1, 0.1, 0.8, 0.8], wcs=wcs)
fig.add_axes(ax)
ax.set_xlim(1, -1)
ax.set_ylim(-1, 1)
ax.grid(color='gray', alpha=0.5)
ax.coords['ra'].set_ticks(color='red', size=20)
ax.coords['dec'].set_ticks(color='red', size=20)
fig.savefig('test.png', dpi=72)

test

@@ -318,13 +327,18 @@ def _update_ticks(self, renderer):
pixel0 = spine.data
world0 = spine.world[:,self.coord_index]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Isn't this line a no-op because world0 is assigned to again in the following line?

Copy link
Owner

Choose a reason for hiding this comment

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

Indeed - we should try and figure out when this was introduced (I'll take a look at git blame)

@astrofrog
Copy link
Owner

This is a great idea, thanks! Will leave a few comments below. I think we should then wait for #41 to be merged since it adds image testing, then we can add a regression test for this.

# See:
# http://matplotlib.org/users/transforms_tutorial.html#the-transformation-pipeline
transLimits = self.parent_axes.transLimits
invertedTransLimits = transLimits.inverted()
Copy link
Owner

Choose a reason for hiding this comment

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

If these are always going to be affine transformations with no rotation, I wonder whether it would be more efficient to simply find out what the scale from axes to data coordinates is and multiply/divide by these later on?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If these are always going to be affine transformations with no rotation, I wonder whether it would be more efficient to simply find out what the scale from axes to data coordinates is and multiply/divide by these later on?

Yes, but it would be clearer and nearly as efficient to pass all of the ticks to transform() as one big vector.

You might be able to eliminate the multiple transformation objects by using a blended transform.

@astrofrog
Copy link
Owner

@lpsinger - could you rebase this now that #41 has been merged? Also, it would be great if you could add the example you have above as an image test. To do this, you need to add a test to test_image.py then generate the reference image by running the tests with:

py.test --generate-reference True wcsaxes/tests/test_image.py

If you don't have time to do this, we could merge it as-is and I or @anizami could add the image test.

@anizami
Copy link
Collaborator

anizami commented Jun 16, 2014

@lpsinger @astrofrog - I actually made a minor change so you can just generate the reference image by py.test --generate-reference wcsaxes/tests/test_image.py

@astrofrog
Copy link
Owner

@anizami - great!

@lpsinger
Copy link
Contributor Author

@lpsinger - could you rebase this now that #41 has been merged?

Updated via force push.

@lpsinger
Copy link
Contributor Author

Also, it would be great if you could add the example you have above as an image test.

Matplotlib tries to limit the number of test images to keep the repository size small. Is it OK if I make one image test that handles #43, #44, and #46?

@astrofrog
Copy link
Owner

@lpsinger - I think that #43/#44 and #46 should be separate since they are testing very different things. However, I think the reference images won't be too big as long as you don't actually plot an image.

@lpsinger
Copy link
Contributor Author

@lpsinger - I think that #43/#44 and #46 should be separate since they are testing very different things. However, I think the reference images won't be too big as long as you don't actually plot an image.

Alright, shall I make one test for #43/#44 and then rebase #43/#44 into one PR?

@astrofrog
Copy link
Owner

@lpsinger - that'd be great, thanks!

@lpsinger
Copy link
Contributor Author

@lpsinger @astrofrog - I actually made a minor change so you can just generate the reference image by py.test --generate-reference wcsaxes/tests/test_image.py

I have a unit test, but I cannot generate a reference image. I get:

$ py.test-2.7 --generate-reference True wcsaxes/tests/test_image.py
usage: py.test-2.7 [options] [file_or_dir] [file_or_dir] [...]
py.test-2.7: error: unrecognized arguments: --generate-reference

EDIT: never mind, figured it out.

@astrofrog
Copy link
Owner

@lpsinger - just to check, are you still planning on adding a reference image to this PR? It would be great to have a regression test to avoid this issue creeping back in future.

@lpsinger
Copy link
Contributor Author

@lpsinger - just to check, are you still planning on adding a reference image to this PR?

Yes, I thought I would wait until after #49 was closed.

@astrofrog
Copy link
Owner

@lpsinger - ok, #49 has now been merged, so feel free to add a test here - thanks!

This handles cases when a plot extends over a small number of pixels in
the WCS projection, but a large distance or number of pixels in display
coordinates. Addresses astrofrog#45.
@lpsinger
Copy link
Contributor Author

@lpsinger - ok, #49 has now been merged, so feel free to add a test here - thanks!

Done. By the way, I had to dig a little in the source code to remind myself of the --generate-reference option. Could you document it, please?

@anizami
Copy link
Collaborator

anizami commented Jun 18, 2014

@astrofrog - Since this options seems more relevant for developers, where do you recommend we document this?

@lpsinger
Copy link
Contributor Author

@astrofrog - Since this options seems more relevant for developers, where do you recommend we document this?

If your goal is to eventually put this code into Astropy, you could imitate their Developer Documentation section, which includes Testing Guidelines.

@astrofrog
Copy link
Owner

Thanks @lpsinger!

astrofrog added a commit that referenced this pull request Jun 19, 2014
Compute tick angles by advancing a small amount in axes coordinates
@astrofrog astrofrog merged commit 7c49b4d into astrofrog:master Jun 19, 2014
@astrofrog
Copy link
Owner

@anizami - can you open an issue for now to remind ourselves to document the --generate-reference and we can chat about it later?

@anizami
Copy link
Collaborator

anizami commented Jun 19, 2014

@astrofrog - Sure!

@astrofrog
Copy link
Owner

@lpsinger - it looks like this doesn't work properly if the axes are not square. Let me know if you have time to look into how to fix this today. If not, then I can look into it.

@lpsinger
Copy link
Contributor Author

@lpsinger - it looks like this doesn't work properly if the axes are not square.

Yes, I'll take a look. Do you have an example script?

@astrofrog
Copy link
Owner

@lpsinger - yes, simply take your example and make the figure e.g. 6x3:

w = WCS()
w.wcs.ctype = ['RA---TAN', 'DEC--TAN']
w.wcs.crval = [90, 70]
w.wcs.cdelt = [16, 16]
w.wcs.crpix = [1, 1]
w.wcs.radesys = 'ICRS'
w.wcs.equinox = 2000.0
fig = plt.figure(figsize=(6, 3))
ax = WCSAxes(fig, [0.1, 0.1, 0.8, 0.8], wcs=w)
fig.add_axes(ax)
ax.set_xlim(1, -1)
ax.set_ylim(-1, 1)
ax.grid(color='gray', alpha=0.5, linestyle='solid')
ax.coords['ra'].set_ticks(color='red', size=20)
ax.coords['dec'].set_ticks(color='red', size=20)
fig.savefig('angles.png')

angles

astrofrog added a commit that referenced this pull request Nov 28, 2016
Compute tick angles by advancing a small amount in axes coordinates
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants