Add --slow option for tests - #2317

👍 Though really those tests should just use a smaller number of coordinates, probably with some thought about them being near poles, etc. Anyway, I think even just marking those tests will be useful incentive to try to speed them up!

cc @eteq

Thank goodness!

And yeah, I think maybe the coordinates accuracy test could be broken up between a minimal subset, and running the full suite which would be more necessary when testing changes that directly affects the coordinates module.

👍 from me on this, at least in some form. @embray's idea is in principal a good idea, but it's not really clear how that "minimal subset" would work - the accuracy tests are very simple, and just use a reference file of the "right answer".

Perhaps the key is to have a "slow" version that runs them all, and a "fast" version that just does the first few coordinate sets tests instead of all 200-or-so per system ? @cdeil - feel free to take a shot at that, or just flagging them all "slow" and we can break out faster ones later or something.

That was exactly what I had in mind.

Of course, we could just speed up astropy.coordinates 😀

Anyway, I agree that we can probably reduce the number of tests to 20 for the regular tests, and do the rest only with some kind of --runslow option. We can just pick the 20 first ones since they are random.

As an aside: Any idea why the astropy/io/fits/tests/test_image.py::TestImageFunctions::test_lossless_gzip_compression is so much slower on a Mac than Linux? Should we create an issue for that?

That is really weird. Is it reproducible or was it just a glitch?

I've added the slow test option and changed the slow coordinate tests.

By default the tests now run a minute faster:

$ time python setup.py test -P coordinates -V -a --durations=10
===================================================================== slowest 10 test durations ======================================================================
1.19s call     astropy/coordinates/tests/accuracy/test_fk4_no_e_fk5.py::test_fk4_no_e_fk5_fast
0.83s call     astropy/coordinates/tests/accuracy/test_galactic_fk4.py::test_galactic_fk4_fast
0.81s call     astropy/coordinates/tests/accuracy/test_icrs_fk5.py::test_icrs_no_e_fk5_fast
0.15s call     astropy/coordinates/tests/accuracy/test_fk4_no_e_fk4.py::test_fk4_no_e_fk5
0.07s call     astropy/coordinates/tests/test_api.py::test_distances_scipy
0.07s call     astropy/coordinates/tests/test_transformations.py::test_precession
0.06s setup    astropy/coordinates/tests/accuracy/test_fk4_no_e_fk5.py::test_fk4_no_e_fk5_fast
0.06s call     astropy/coordinates/tests/test_api.py::test_create_coordinate
0.05s call     astropy/coordinates/tests/test_transformations.py::test_m31_coord_transforms[fromsys3-tosys3-fromcoo3-tocoo3]
0.04s call     astropy/coordinates/tests/test_transformations.py::test_obstime

But the full accuracy tests are still available if you use the `--slow`` option:

$ time python setup.py test -P coordinates -V --slow -a --durations=10
===================================================================== slowest 10 test durations ======================================================================
24.89s call     astropy/coordinates/tests/accuracy/test_fk4_no_e_fk5.py::test_fk4_no_e_fk5_slow
17.34s call     astropy/coordinates/tests/accuracy/test_galactic_fk4.py::test_galactic_fk4_slow
16.91s call     astropy/coordinates/tests/accuracy/test_icrs_fk5.py::test_icrs_no_e_fk5_slow
1.23s call     astropy/coordinates/tests/accuracy/test_fk4_no_e_fk5.py::test_fk4_no_e_fk5_fast
0.86s call     astropy/coordinates/tests/accuracy/test_galactic_fk4.py::test_galactic_fk4_fast
0.85s call     astropy/coordinates/tests/accuracy/test_icrs_fk5.py::test_icrs_no_e_fk5_fast
0.15s call     astropy/coordinates/tests/accuracy/test_fk4_no_e_fk4.py::test_fk4_no_e_fk5
0.06s call     astropy/coordinates/tests/test_api.py::test_distances_scipy
0.06s call     astropy/coordinates/tests/test_transformations.py::test_precession
0.06s setup    astropy/coordinates/tests/accuracy/test_fk4_no_e_fk5.py::test_fk4_no_e_fk5_slow

@embray @mdboom Yes, the astropy/io/fits/tests/test_image.py::TestImageFunctions::test_lossless_gzip_compression test is consistently slow on my Macbook (using Macports). Do you have access to a Mac to debug this?

Here's the new top top slowest Astropy tests on my Macbook:

11.97s call     astropy/io/fits/tests/test_image.py::TestImageFunctions::test_lossless_gzip_compression
4.98s call     astropy/io/fits/tests/test_table.py::TestTableFunctions::test_copy_vla
3.31s call     astropy/utils/tests/test_timer.py::test_timer
3.01s call     astropy/vo/samp/tests/test_hub_script.py::test_hub_script
2.82s call     astropy/io/fits/tests/test_table.py::TestTableFunctions::test_variable_length_columns
2.82s call     astropy/io/fits/tests/test_table.py::TestTableFunctions::test_getdata_vla
2.67s call     astropy/io/fits/tests/test_image.py::TestImageFunctions::test_open_scaled_in_update_mode_compressed
2.42s call     astropy/units/tests/test_units.py::test_compose_fractional_powers
2.26s call     astropy/units/tests/test_units.py::test_compose_fractional_powers
2.05s call     astropy/io/fits/tests/test_image.py::TestImageFunctions::test_open_scaled_in_update_mode

Note that there are quite a few parametrized tests where each parameter set is counted as a separate test and thus don't show up with the pytest --durations option.

Can someone please review this?
I implemented slow basically by search and replace for remote_data without trying to understand the code in detail, so this definitely needs to be checked.

👏

The FITS tests that are slow for you are pretty I/O intensive. That could be a bottleneck on your machine.

I've made a new issue #2345 for the slow FITS unit tests on Mac.

Any other suggestions for this PR?

@cdeil - the APE5 changes are going to conflict with this quite a bit, unfortunately, because it's a fairly drastic change of how these accuracy tests are actually invoked... so I think we should wait until that gets in to finalize this (but because this is already in as a PR, it's fine to do that after the official feature freeze).

If it helps, the APE5 branch also now runs only a subset of the accuracy tests, so it's about 10x faster. But that could be changed once this is combined in.

(And if you want to see what I mean about the changes, you can take a look at https://github.com/eteq/astropy/tree/coordinates-ape5 )

This is now in #2422 - the accuracy tests are dramatically sped up by just only running a fraction of them at a time.

@cdeil - do you consider this taken care of now that #2422 is in, or do you still want to add this for other tests?

Is this still needed? Now that the coordinate tests are faster I'd actually vote that we don't include it, just to keep some kind of limit on the number of options we are adding (feature creep). One usually never has to really run the whole test suite anyway, since we can use the -P and -t options to run only parts of the test suite.

The use case I have for it is to run "integration tests" for Gammapy, i.e. small analysis chains that process a test dataset and produce some high-level analysis result. Using the normal testing setup for that is convenient, because really all that's different about these tests compared to unit tests is that they are slow and don't need to run for every commit on travis-ci.

@cdeil - ok, in that case do you want to go ahead and rebase this? Also, note that the changes to the astropy_test class should not be made here, but in astropy-helpers.

I'd still be fine with including this if you want to rebase it (or I can, and manually merge). I think the changelog entry just needs to be moved.

Postponing for now...

@aphearin asked for this feature again on astropy-dev today:
https://groups.google.com/forum/#!topic/astropy-dev/xghxhxseZFw

There were some concerns if this is useful enough to be included, and this got sidetracked a bit by discussions on performance issues in Astropy.
But looking back trough the comments it seems that there's a good chance this could get merged.
So I'll make a new PR against astropy-helpers with this functionality in the coming days and link to it from here.

I'll follow this thread in the coming days in case there is some hangup. In case this can continues to get kicked down the road in astropy_helpers, it's no sweat, I'd just want to know how to make the necessary modifications within my own package (since my simple decorator solution failed). But of course if it gets implemented within the helpers, from my perspective that is ideal. Either way, thanks for the help @cdeil

I didn't realize there was any objection to this. I think it's useful to have.

As for astropy-helpers maybe leave it alone. This is good incentive for me to rework how options are added to the ./setup.py test command so that it can be determined by what version of the astropy test runner a project is using, and doesn't have to be so tightly bound to it (there are already some improvements on that front in that it won't crash if you use mismatched versions, but it will be better if the command options are determined by introspection).

As for astropy-helpers maybe leave it alone. This is good incentive for me to rework ...

@embray Great! Please mention it here when you file an issue for astropy-helpers or make a PR.

Yeah, I'll try to get that cleaned up today. It's long overdue.

Just checking on the status of the --slow feature. I've recently accumulated a large number of unavoidably slow tests in Halotools, and so this is increasingly a problem I either need to solve or devise a temporary workaround for.

CC @cdeil , @embray

I think this mostly just depends on me finishing #4020. I think there's more work left for me to do there but I got sidetracked.

Hey @embray - just checking on the status of this now that #4020 is merged. Did this slow-testing option get integrated into Astropy v1.0.5?

No, and #4020 was only for Astropy 1.1 too. But I'd be happy to try to get this integrated for 1.1...

Just a thought - if this goes ahead, the option should be either --run-slow or --skip-slow, otherwise it's too ambiguous.

Just for the record, as I pointed out to @aphearin it's super easy to add one's own custom markers with py.test https://pytest.org/latest/example/markers.html

One can then use the -m option for running only tests with that marker, or only tests without it. Since our setup.py test doesn't support that option natively it has to be passed through to py.test using the --args, like --args="-m 'not slow'".

Or even more simply using the recipe this PR was originally based on: http://pytest.org/latest/example/simple.html?highlight=mark%20slow#control-skipping-of-tests-according-to-command-line-option This version also makes it so that the "slow" tests are skipped by default.

There's no reason py.test recipes like this can't be used with the astropy test runner (if it doesn't work for some reason that's probably a bug on our end). The only reason for this PR is to add an argument to the test runner to make it more convenient to enable slow tests (which I'm fine with doing, now that it's easier to keep the test runner and then ./setup.py test command in sync).

I'm closing this old PR.

@embray points out how to achieve the same thing directly with pytest, and I agree the value of re-exposing it as a python setup.py test option is limited.

On my machine, currently with Astropy master the tests run for 228.88 seconds and these are the slowest:

========================================================================= slowest 100 test durations =========================================================================
4.57s call     lib.macosx-10.12-x86_64-3.5/astropy/table/tests/test_mixin.py::test_io_ascii_write
4.18s call     lib.macosx-10.12-x86_64-3.5/astropy/units/tests/test_units.py::test_compose_fractional_powers
3.77s call     lib.macosx-10.12-x86_64-3.5/astropy/units/tests/test_units.py::test_compose_cgs_to_si[Bi]
3.60s call     lib.macosx-10.12-x86_64-3.5/astropy/units/tests/test_units.py::test_compose_cgs_to_si[Fr]
3.33s call     lib.macosx-10.12-x86_64-3.5/astropy/utils/tests/test_timer.py::test_timer
3.06s call     lib.macosx-10.12-x86_64-3.5/astropy/units/tests/test_units.py::test_compose_cgs_to_si[statA]
3.03s call     lib.macosx-10.12-x86_64-3.5/astropy/vo/samp/tests/test_hub.py::test_SAMPHubServer_run_repeated
3.02s call     lib.macosx-10.12-x86_64-3.5/astropy/vo/samp/tests/test_hub_script.py::test_hub_script
2.60s call     docs/units/decomposing_and_composing.rst
2.51s call     lib.macosx-10.12-x86_64-3.5/astropy/units/tests/test_units.py::test_complex_compose
2.44s call     lib.macosx-10.12-x86_64-3.5/astropy/io/fits/tests/test_image.py::TestCompressedImage::test_open_scaled_in_update_mode_compressed
2.25s call     lib.macosx-10.12-x86_64-3.5/astropy/wcs/tests/extension/test_extension.py::test_wcsapi_extension
2.06s call     lib.macosx-10.12-x86_64-3.5/astropy/io/fits/tests/test_image.py::TestImageFunctions::test_open_scaled_in_update_mode
1.76s call     lib.macosx-10.12-x86_64-3.5/astropy/coordinates/tests/test_angles.py::test_repr_latex
1.68s call     lib.macosx-10.12-x86_64-3.5/astropy/convolution/tests/test_discretize.py::test_subpixel_gauss_2D
1.66s call     lib.macosx-10.12-x86_64-3.5/astropy/io/ascii/tests/test_c_reader.py::test_many_columns[True]
1.41s call     lib.macosx-10.12-x86_64-3.5/astropy/table/tests/test_mixin.py::test_hstack[masked]
1.39s call     lib.macosx-10.12-x86_64-3.5/astropy/table/tests/test_mixin.py::test_hstack[unmasked]
1.31s call     lib.macosx-10.12-x86_64-3.5/astropy/table/tests/test_mixin.py::test_hstack[subclass]
1.17s call     lib.macosx-10.12-x86_64-3.5/astropy/io/ascii/tests/test_c_reader.py::test_many_columns[False]
1.13s call     lib.macosx-10.12-x86_64-3.5/astropy/table/tests/test_info.py::test_table_info_attributes[subclass]
1.12s call     lib.macosx-10.12-x86_64-3.5/astropy/table/tests/test_info.py::test_table_info_attributes[unmasked]
1.11s call     lib.macosx-10.12-x86_64-3.5/astropy/table/tests/test_mixin.py::test_join[masked]
1.11s call     lib.macosx-10.12-x86_64-3.5/astropy/table/tests/test_mixin.py::test_join[subclass]
1.09s call     lib.macosx-10.12-x86_64-3.5/astropy/table/tests/test_mixin.py::test_join[unmasked]
1.06s call     lib.macosx-10.12-x86_64-3.5/astropy/nddata/tests/test_nduncertainty.py::test_for_leak_with_uncertainty
1.01s call     lib.macosx-10.12-x86_64-3.5/astropy/vo/samp/tests/test_hub.py::test_SAMPHubServer_run
1.01s call     lib.macosx-10.12-x86_64-3.5/astropy/io/fits/tests/test_groups.py::TestGroupsFunctions::test_open_groups_in_update_mode
1.01s call     lib.macosx-10.12-x86_64-3.5/astropy/io/fits/tests/test_image.py::TestCompressedImage::test_open_comp_image_in_update_mode
1.00s call     lib.macosx-10.12-x86_64-3.5/astropy/io/fits/tests/test_image.py::TestCompressedImage::test_lossless_gzip_compression

Probably the Astropy tests could be made faster, my experience is that people very often choose test datasets that could be smaller and still test the exact same thing.
But nothing stands out particularly, so I won't go have a look now.