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

added remote-data decorator to BaseImageTests class #67

Merged
merged 5 commits into from
Jul 2, 2014

Conversation

anizami
Copy link
Collaborator

@anizami anizami commented Jul 1, 2014

I had to add the decorator to the BaseImageTests class because the images were downloaded in the setup_class method. Now python setup.py test skips all the tests in test_images.py and test_transform_coord_meta.py. Is that a problem?

@astrofrog
Copy link
Owner

@anizami - maybe it would make sense instead to not pre-download the files in the setup_class, but only when needed directly in the tests. Then you can have more control over which tests to skip. We used to have it in setup_class when the code to get the files was more complex, but now we can use the datasets module, which caches, there's no real benefit to setting them in setup_class.

@astrofrog
Copy link
Owner

@anizami - thanks! It looks like some more tests make use of _image1_hdu and so on - see Travis failures

@anizami
Copy link
Collaborator Author

anizami commented Jul 1, 2014

@astrofrog - Yeah I noticed. It's actually a bit trickier than just calling datasets inside a test. I've noticed that it doesn't cache unless datasets is called inside setup_class..trying to fix that right now.

@anizami
Copy link
Collaborator Author

anizami commented Jul 1, 2014

@astrofrog - Is there a way I can check inside a function whether or not --remote-data is used?

@astrofrog
Copy link
Owner

@anizami - good question, I'm not sure! Weird that the caching isn't working, it could be because the tests each have their own cache to ensure no overlap. Is it really slow if the cache doesn't get used?

@anizami
Copy link
Collaborator Author

anizami commented Jul 1, 2014

@astrofrog - Yep, annoyingly slow

@astrofrog
Copy link
Owner

@anizami - ok, if there is no easy way to do it, then leave the decorator on the class as before, and it's ok if it skips all image tests.

@anizami
Copy link
Collaborator Author

anizami commented Jul 1, 2014

@astrofrog - hmm okay. It's not as satisfying though. By the way, test_custom_frame in test_frame.py also currently uses datasets.msx_hdu() which doesn't cache either. I'm going to change that too so it caches.

@@ -67,9 +68,8 @@ def test_image_plot(self, generate):
# Test for overlaying contours on images
@remote_data
Copy link
Owner

Choose a reason for hiding this comment

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

Can we remove @remote_data from these tests now?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, I'll do that.

@astrofrog
Copy link
Owner

@anizami - I think the solution is going to be that since many of the tests don't need the actual image data, we could store the FITS headers in the repository and only use datasets. in tests when we need the actual image data. We can chat about this on the hangout.

@anizami
Copy link
Collaborator Author

anizami commented Jul 1, 2014

@astrofrog - turns out we do use hdu.data when setting xlim and ylim in the images. Now what should I do?

@astrofrog
Copy link
Owner

@anizami - you can just hard-code the sizes - just print out the shapes and enter them manually :)

@anizami
Copy link
Collaborator Author

anizami commented Jul 1, 2014

@astrofrog - ah of course..

@anizami
Copy link
Collaborator Author

anizami commented Jul 1, 2014

@astrofrog - by the way, do you have a preference about which directory to keep the header files in?

@astrofrog
Copy link
Owner

Maybe you can make a data sub-folder inside tests?

@anizami
Copy link
Collaborator Author

anizami commented Jul 1, 2014

Sure

@anizami
Copy link
Collaborator Author

anizami commented Jul 1, 2014

@astrofrog - the contour_overlay image uses hdu.data in the ax.contour method but this is actually the only test that uses the data. Turns out the cube slicing tests don't need the data too. I'm thinking of just marking contour_overlay test as remote_data and since the file isn't that large, it's okay that it doesn't cache. Does that sound like an okay idea to you?

@anizami
Copy link
Collaborator Author

anizami commented Jul 1, 2014

Summary: all the headers are now in the data directory, only one test is now marked with the remote_data decorator, python setup.py test doesn't give any errors and Travis is passing.

@@ -16,13 +16,14 @@


class BaseImageTests(object):

# TODO: move header files to data directory, add them to config files so they get copied over
Copy link
Owner

Choose a reason for hiding this comment

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

Can this be removed?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ah forgot about this.

@astrofrog
Copy link
Owner

Looks great, and works nicely!

astrofrog added a commit that referenced this pull request Jul 2, 2014
added remote-data decorator to BaseImageTests class
@astrofrog astrofrog merged commit 03378f1 into astrofrog:master Jul 2, 2014
@anizami anizami deleted the remote_data-decorator branch July 4, 2014 07:02
astrofrog added a commit that referenced this pull request Nov 28, 2016
added remote-data decorator to BaseImageTests class
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.

2 participants