astroquery icon indicating copy to clipboard operation
astroquery copied to clipboard

NOIRLab advanced search

Open pothiers opened this issue 5 years ago • 15 comments
trafficstars

Adds capability to search ANY metadata in Archived FITS file headers (in any HDU).

pothiers avatar Apr 15 '20 23:04 pothiers

Milestone needs to be set on this (0.4.1). If its possible for me to do it, let me know how.

pothiers avatar Apr 16 '20 14:04 pothiers

Hello @pothiers! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! :beers:

Comment last updated at 2020-05-25 16:08:31 UTC

pep8speaks avatar Apr 16 '20 17:04 pep8speaks

Codecov Report

Merging #1701 into master will increase coverage by 0.29%. The diff coverage is 28.30%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1701      +/-   ##
==========================================
+ Coverage   62.67%   62.97%   +0.29%     
==========================================
  Files         189      195       +6     
  Lines       15192    15295     +103     
==========================================
+ Hits         9522     9632     +110     
+ Misses       5670     5663       -7     
Impacted Files Coverage Δ
astroquery/noirlab/__init__.py 100.00% <ø> (ø)
astroquery/noirlab/core.py 40.00% <28.30%> (-18.98%) :arrow_down:
astroquery/alma/core.py 35.42% <0.00%> (-0.16%) :arrow_down:
astroquery/mast/__init__.py 100.00% <0.00%> (ø)
astroquery/mast/fpl.py
astroquery/mast/tesscut.py
astroquery/mast/utils.py 76.08% <0.00%> (ø)
astroquery/mast/services.py 78.03% <0.00%> (ø)
astroquery/mast/collections.py 92.10% <0.00%> (ø)
astroquery/mast/cutouts.py 78.76% <0.00%> (ø)
... and 6 more

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 272bfb1...75e72a8. Read the comment docs.

codecov[bot] avatar Apr 16 '20 18:04 codecov[bot]

I don't think the documentation is complete. I must have lost something in the shuffle of three PRs plus one previous version of this. Looking into it. (It should have lots of extra methods in the API)

pothiers avatar Apr 21 '20 19:04 pothiers

@pothiers - It would be nice to have more substantial user facing documentation, with more examples. If you prefer it can be added in a follow-up PR once this is merged. E.g. the nice test coverage could be worked into a narrative docs, too.

Also, I see 3 failures that we won't see on CI as we don't run the remote tests for PRs to limit the stress on remote servers. For the record these are the failures I see.

Note for @keflavich and @ceb8 - we need to double check the test locally before merging this.

======================================================= FAILURES =======================================================
________________________________________ TestNoirlabClass.test_core_file_fields ________________________________________

self = <astroquery.noirlab.tests.test_noirlab_remote.TestNoirlabClass object at 0x135f57990>

    def test_core_file_fields(self):
        """List the available CORE FILE fields."""
        r = Noirlab().core_fields()
        actual = r
        print(f'DBG: test_core_file_fields={actual}')
        expected = exp.core_file_fields
>       assert actual == expected
E       AssertionError: assert [{'Field': 'a...loat64'}, ...] == {'archive_fil... type.'], ...}
E         Use -v to get the full diff

astroquery/noirlab/tests/test_noirlab_remote.py:93: AssertionError
------------------------------------------------- Captured stdout call -------------------------------------------------
DBG: test_core_file_fields=[{'Field': 'archive_filename', 'Type': 'str'}, {'Field': 'caldat', 'Type': 'datetime64'}, {'Field': 'date_obs_max', 'Type': 'datetime64'}, {'Field': 'date_obs_min', 'Type': 'datetime64'}, {'Field': 'dec_max', 'Type': 'np.float64'}, {'Field': 'dec_min', 'Type': 'np.float64'}, {'Field': 'depth', 'Type': 'np.float64'}, {'Field': 'exposure', 'Type': 'np.float64'}, {'Field': 'filesize', 'Type': 'np.int64'}, {'Field': 'ifilter', 'Type': 'category'}, {'Field': 'instrument', 'Type': 'category'}, {'Field': 'md5sum', 'Type': 'str'}, {'Field': 'obs_mode', 'Type': 'category'}, {'Field': 'obs_type', 'Type': 'category'}, {'Field': 'original_filename', 'Type': 'str'}, {'Field': 'proc_type', 'Type': 'category'}, {'Field': 'prod_type', 'Type': 'category'}, {'Field': 'proposal', 'Type': 'category'}, {'Field': 'ra_max', 'Type': 'np.float64'}, {'Field': 'ra_min', 'Type': 'np.float64'}, {'Field': 'release_date', 'Type': 'datetime64'}, {'Field': 'seeing', 'Type': 'np.float64'}, {'Field': 'site', 'Type': 'category'}, {'Field': 'survey', 'Type': 'category'}, {'Field': 'telescope', 'Type': 'category'}, {'Field': 'updated', 'Type': 'datetime64'}]
________________________________________ TestNoirlabClass.test_core_hdu_fields _________________________________________

self = <astroquery.noirlab.tests.test_noirlab_remote.TestNoirlabClass object at 0x135f1df10>

    def test_core_hdu_fields(self):
        """List the available CORE HDU fields."""
        r = Noirlab(which='hdu').core_fields()
        actual = r
        print(f'DBG: test_core_hdu_fields={actual}')
        expected = exp.core_hdu_fields
>       assert actual == expected
E       AssertionError: assert [{'Field': 'b...: 'str'}, ...] == {'boundary': ... match'], ...}
E         Use -v to get the full diff

astroquery/noirlab/tests/test_noirlab_remote.py:134: AssertionError
------------------------------------------------- Captured stdout call -------------------------------------------------
DBG: test_core_hdu_fields=[{'Field': 'boundary', 'Type': 'str'}, {'Field': 'dec', 'Type': 'np.float64'}, {'Field': 'dec_range', 'Type': 'str'}, {'Field': 'fitsfile', 'Type': 'str'}, {'Field': 'fitsfile__archive_filename', 'Type': 'str'}, {'Field': 'fitsfile__caldat', 'Type': 'str'}, {'Field': 'fitsfile__date_obs_max', 'Type': 'str'}, {'Field': 'fitsfile__date_obs_min', 'Type': 'str'}, {'Field': 'fitsfile__dec_max', 'Type': 'str'}, {'Field': 'fitsfile__dec_min', 'Type': 'str'}, {'Field': 'fitsfile__depth', 'Type': 'str'}, {'Field': 'fitsfile__exposure', 'Type': 'str'}, {'Field': 'fitsfile__filesize', 'Type': 'str'}, {'Field': 'fitsfile__ifilter', 'Type': 'str'}, {'Field': 'fitsfile__instrument', 'Type': 'str'}, {'Field': 'fitsfile__md5sum', 'Type': 'str'}, {'Field': 'fitsfile__obs_mode', 'Type': 'str'}, {'Field': 'fitsfile__obs_type', 'Type': 'str'}, {'Field': 'fitsfile__original_filename', 'Type': 'str'}, {'Field': 'fitsfile__proc_type', 'Type': 'str'}, {'Field': 'fitsfile__prod_type', 'Type': 'str'}, {'Field': 'fitsfile__proposal', 'Type': 'str'}, {'Field': 'fitsfile__ra_max', 'Type': 'str'}, {'Field': 'fitsfile__ra_min', 'Type': 'str'}, {'Field': 'fitsfile__release_date', 'Type': 'str'}, {'Field': 'fitsfile__seeing', 'Type': 'str'}, {'Field': 'fitsfile__site', 'Type': 'str'}, {'Field': 'fitsfile__survey', 'Type': 'str'}, {'Field': 'fitsfile__telescope', 'Type': 'str'}, {'Field': 'fitsfile__updated', 'Type': 'str'}, {'Field': 'hdu_idx', 'Type': 'np.int64'}, {'Field': 'id', 'Type': 'str'}, {'Field': 'ra', 'Type': 'np.float64'}, {'Field': 'ra_range', 'Type': 'str'}, {'Field': 'updated', 'Type': 'datetime64'}]
__________________________________________ TestNoirlabClass.test_categoricals __________________________________________

self = <astroquery.noirlab.tests.test_noirlab_remote.TestNoirlabClass object at 0x135f47750>

    def test_categoricals(self):
        """List categories."""
        r = Noirlab().categoricals()
        actual = r
        print(f'DBG: test_categoricals={actual}')
        expected = exp.categoricals
>       assert actual == expected
E       AssertionError: assert {'instruments...1', ...], ...} == {'collections...d', ...], ...}
E         Omitting 7 identical items, use -vv to show
E         Right contains 1 more item:
E         {'collections': []}
E         Use -v to get the full diff

astroquery/noirlab/tests/test_noirlab_remote.py:169: AssertionError
------------------------------------------------- Captured stdout call -------------------------------------------------
DBG: test_categoricals={'instruments': ['(p)odi', '90prime', 'andicam', 'arcoiris', 'arcon', 'bench', 'ccd_imager', 'chiron', 'cosmos', 'decam', 'echelle', 'falmingos', 'flamingos', 'goodman', 'goodman spectrograph', 'gtcam', 'hdi', 'ice', 'ispi', 'kosmos', 'minimo/ice', 'mop/ice', 'mosaic', 'mosaic3', 'mosaic_1', 'mosaic_1_1', 'mosaic_2', 'newfirm', 'osiris', 'sami', 'soi', 'spartan', 'spartan ir camera', 'triplespec', 'wfc', 'whirc', 'wildfire', 'y4kcam'], 'obsmodes': [], 'proctypes': ['instcal', 'mastercal', 'nota', 'projected', 'raw', 'resampled', 'skysub', 'stacked'], 'prodtypes': ['dqmask', 'expmap', 'graphics (size)', 'image', 'image 2nd version 1', 'image1', 'nota', 'resampled', 'weight', 'wtmap'], 'sites': ['cp', 'ct', 'kp', 'lp'], 'surveys': [], 'telescopes': ['bok23m', 'ct09m', 'ct13m', 'ct15m', 'ct1m', 'ct4m', 'ctlab', 'kp09m', 'kp21m', 'kp35m', 'kp4m', 'kpcf', 'lp25m', 'soar', 'wiyn']}
============================================ 3 failed, 12 passed in 17.38s =============================================

bsipocz avatar Apr 23 '20 03:04 bsipocz

@pothiers - It would be nice to have more substantial user facing documentation, with more examples. If you prefer it can be added in a follow-up PR once this is merged. E.g. the nice test coverage could be worked into a narrative docs, too.

Have added more "advanced search" documentation.

pothiers avatar Apr 23 '20 14:04 pothiers

@pothiers - It would be nice to have more substantial user facing documentation, with more examples. If you prefer it can be added in a follow-up PR once this is merged. E.g. the nice test coverage could be worked into a narrative docs, too.

Also, I see 3 failures that we won't see on CI as we don't run the remote tests for PRs to limit the stress on remote servers. For the record these are the failures I see.

Note for @keflavich and @ceb8 - we need to double check the test locally before merging this.

======================================================= FAILURES =======================================================
________________________________________ TestNoirlabClass.test_core_file_fields ________________________________________

self = <astroquery.noirlab.tests.test_noirlab_remote.TestNoirlabClass object at 0x135f57990>

    def test_core_file_fields(self):
        """List the available CORE FILE fields."""
        r = Noirlab().core_fields()
        actual = r
        print(f'DBG: test_core_file_fields={actual}')
        expected = exp.core_file_fields
>       assert actual == expected
E       AssertionError: assert [{'Field': 'a...loat64'}, ...] == {'archive_fil... type.'], ...}
E         Use -v to get the full diff

astroquery/noirlab/tests/test_noirlab_remote.py:93: AssertionError
------------------------------------------------- Captured stdout call -------------------------------------------------
DBG: test_core_file_fields=[{'Field': 'archive_filename', 'Type': 'str'}, {'Field': 'caldat', 'Type': 'datetime64'}, {'Field': 'date_obs_max', 'Type': 'datetime64'}, {'Field': 'date_obs_min', 'Type': 'datetime64'}, {'Field': 'dec_max', 'Type': 'np.float64'}, {'Field': 'dec_min', 'Type': 'np.float64'}, {'Field': 'depth', 'Type': 'np.float64'}, {'Field': 'exposure', 'Type': 'np.float64'}, {'Field': 'filesize', 'Type': 'np.int64'}, {'Field': 'ifilter', 'Type': 'category'}, {'Field': 'instrument', 'Type': 'category'}, {'Field': 'md5sum', 'Type': 'str'}, {'Field': 'obs_mode', 'Type': 'category'}, {'Field': 'obs_type', 'Type': 'category'}, {'Field': 'original_filename', 'Type': 'str'}, {'Field': 'proc_type', 'Type': 'category'}, {'Field': 'prod_type', 'Type': 'category'}, {'Field': 'proposal', 'Type': 'category'}, {'Field': 'ra_max', 'Type': 'np.float64'}, {'Field': 'ra_min', 'Type': 'np.float64'}, {'Field': 'release_date', 'Type': 'datetime64'}, {'Field': 'seeing', 'Type': 'np.float64'}, {'Field': 'site', 'Type': 'category'}, {'Field': 'survey', 'Type': 'category'}, {'Field': 'telescope', 'Type': 'category'}, {'Field': 'updated', 'Type': 'datetime64'}]
________________________________________ TestNoirlabClass.test_core_hdu_fields _________________________________________

self = <astroquery.noirlab.tests.test_noirlab_remote.TestNoirlabClass object at 0x135f1df10>

    def test_core_hdu_fields(self):
        """List the available CORE HDU fields."""
        r = Noirlab(which='hdu').core_fields()
        actual = r
        print(f'DBG: test_core_hdu_fields={actual}')
        expected = exp.core_hdu_fields
>       assert actual == expected
E       AssertionError: assert [{'Field': 'b...: 'str'}, ...] == {'boundary': ... match'], ...}
E         Use -v to get the full diff

astroquery/noirlab/tests/test_noirlab_remote.py:134: AssertionError
------------------------------------------------- Captured stdout call -------------------------------------------------
DBG: test_core_hdu_fields=[{'Field': 'boundary', 'Type': 'str'}, {'Field': 'dec', 'Type': 'np.float64'}, {'Field': 'dec_range', 'Type': 'str'}, {'Field': 'fitsfile', 'Type': 'str'}, {'Field': 'fitsfile__archive_filename', 'Type': 'str'}, {'Field': 'fitsfile__caldat', 'Type': 'str'}, {'Field': 'fitsfile__date_obs_max', 'Type': 'str'}, {'Field': 'fitsfile__date_obs_min', 'Type': 'str'}, {'Field': 'fitsfile__dec_max', 'Type': 'str'}, {'Field': 'fitsfile__dec_min', 'Type': 'str'}, {'Field': 'fitsfile__depth', 'Type': 'str'}, {'Field': 'fitsfile__exposure', 'Type': 'str'}, {'Field': 'fitsfile__filesize', 'Type': 'str'}, {'Field': 'fitsfile__ifilter', 'Type': 'str'}, {'Field': 'fitsfile__instrument', 'Type': 'str'}, {'Field': 'fitsfile__md5sum', 'Type': 'str'}, {'Field': 'fitsfile__obs_mode', 'Type': 'str'}, {'Field': 'fitsfile__obs_type', 'Type': 'str'}, {'Field': 'fitsfile__original_filename', 'Type': 'str'}, {'Field': 'fitsfile__proc_type', 'Type': 'str'}, {'Field': 'fitsfile__prod_type', 'Type': 'str'}, {'Field': 'fitsfile__proposal', 'Type': 'str'}, {'Field': 'fitsfile__ra_max', 'Type': 'str'}, {'Field': 'fitsfile__ra_min', 'Type': 'str'}, {'Field': 'fitsfile__release_date', 'Type': 'str'}, {'Field': 'fitsfile__seeing', 'Type': 'str'}, {'Field': 'fitsfile__site', 'Type': 'str'}, {'Field': 'fitsfile__survey', 'Type': 'str'}, {'Field': 'fitsfile__telescope', 'Type': 'str'}, {'Field': 'fitsfile__updated', 'Type': 'str'}, {'Field': 'hdu_idx', 'Type': 'np.int64'}, {'Field': 'id', 'Type': 'str'}, {'Field': 'ra', 'Type': 'np.float64'}, {'Field': 'ra_range', 'Type': 'str'}, {'Field': 'updated', 'Type': 'datetime64'}]
__________________________________________ TestNoirlabClass.test_categoricals __________________________________________

self = <astroquery.noirlab.tests.test_noirlab_remote.TestNoirlabClass object at 0x135f47750>

    def test_categoricals(self):
        """List categories."""
        r = Noirlab().categoricals()
        actual = r
        print(f'DBG: test_categoricals={actual}')
        expected = exp.categoricals
>       assert actual == expected
E       AssertionError: assert {'instruments...1', ...], ...} == {'collections...d', ...], ...}
E         Omitting 7 identical items, use -vv to show
E         Right contains 1 more item:
E         {'collections': []}
E         Use -v to get the full diff

astroquery/noirlab/tests/test_noirlab_remote.py:169: AssertionError
------------------------------------------------- Captured stdout call -------------------------------------------------
DBG: test_categoricals={'instruments': ['(p)odi', '90prime', 'andicam', 'arcoiris', 'arcon', 'bench', 'ccd_imager', 'chiron', 'cosmos', 'decam', 'echelle', 'falmingos', 'flamingos', 'goodman', 'goodman spectrograph', 'gtcam', 'hdi', 'ice', 'ispi', 'kosmos', 'minimo/ice', 'mop/ice', 'mosaic', 'mosaic3', 'mosaic_1', 'mosaic_1_1', 'mosaic_2', 'newfirm', 'osiris', 'sami', 'soi', 'spartan', 'spartan ir camera', 'triplespec', 'wfc', 'whirc', 'wildfire', 'y4kcam'], 'obsmodes': [], 'proctypes': ['instcal', 'mastercal', 'nota', 'projected', 'raw', 'resampled', 'skysub', 'stacked'], 'prodtypes': ['dqmask', 'expmap', 'graphics (size)', 'image', 'image 2nd version 1', 'image1', 'nota', 'resampled', 'weight', 'wtmap'], 'sites': ['cp', 'ct', 'kp', 'lp'], 'surveys': [], 'telescopes': ['bok23m', 'ct09m', 'ct13m', 'ct15m', 'ct1m', 'ct4m', 'ctlab', 'kp09m', 'kp21m', 'kp35m', 'kp4m', 'kpcf', 'lp25m', 'soar', 'wiyn']}
============================================ 3 failed, 12 passed in 17.38s =============================================

When I did:

cd ~/astroquery/astroquery/noirlab
pytest --remote-data 

I got no errors. But did get them when doing:

cd ~/astroquery
python setup.py test -P noirlab -R

I thought the two would be equivalent for module only checks. I'll change my checklist to only use the latter test invocation.

pothiers avatar Apr 23 '20 15:04 pothiers

Because there where changes to my PR1701 I merged it locally. I now think I should have rebased. I haven't yet pushed my updates (including conflict resolution). Do I need to somehow untangle to avoid the extra commit logs that will otherwise show? If so, I'll likely need help with the untangling.

pothiers avatar Apr 23 '20 15:04 pothiers

@pothiers - It would be nice to have more substantial user facing documentation, with more examples. If you prefer it can be added in a follow-up PR once this is merged. E.g. the nice test coverage could be worked into a narrative docs, too.

Also, I see 3 failures that we won't see on CI as we don't run the remote tests for PRs to limit the stress on remote servers. For the record these are the failures I see.

Note for @keflavich and @ceb8 - we need to double check the test locally before merging this.

======================================================= FAILURES =======================================================
________________________________________ TestNoirlabClass.test_core_file_fields ________________________________________

self = <astroquery.noirlab.tests.test_noirlab_remote.TestNoirlabClass object at 0x135f57990>

    def test_core_file_fields(self):
        """List the available CORE FILE fields."""
        r = Noirlab().core_fields()
        actual = r
        print(f'DBG: test_core_file_fields={actual}')
        expected = exp.core_file_fields
>       assert actual == expected
E       AssertionError: assert [{'Field': 'a...loat64'}, ...] == {'archive_fil... type.'], ...}
E         Use -v to get the full diff

astroquery/noirlab/tests/test_noirlab_remote.py:93: AssertionError
------------------------------------------------- Captured stdout call -------------------------------------------------
DBG: test_core_file_fields=[{'Field': 'archive_filename', 'Type': 'str'}, {'Field': 'caldat', 'Type': 'datetime64'}, {'Field': 'date_obs_max', 'Type': 'datetime64'}, {'Field': 'date_obs_min', 'Type': 'datetime64'}, {'Field': 'dec_max', 'Type': 'np.float64'}, {'Field': 'dec_min', 'Type': 'np.float64'}, {'Field': 'depth', 'Type': 'np.float64'}, {'Field': 'exposure', 'Type': 'np.float64'}, {'Field': 'filesize', 'Type': 'np.int64'}, {'Field': 'ifilter', 'Type': 'category'}, {'Field': 'instrument', 'Type': 'category'}, {'Field': 'md5sum', 'Type': 'str'}, {'Field': 'obs_mode', 'Type': 'category'}, {'Field': 'obs_type', 'Type': 'category'}, {'Field': 'original_filename', 'Type': 'str'}, {'Field': 'proc_type', 'Type': 'category'}, {'Field': 'prod_type', 'Type': 'category'}, {'Field': 'proposal', 'Type': 'category'}, {'Field': 'ra_max', 'Type': 'np.float64'}, {'Field': 'ra_min', 'Type': 'np.float64'}, {'Field': 'release_date', 'Type': 'datetime64'}, {'Field': 'seeing', 'Type': 'np.float64'}, {'Field': 'site', 'Type': 'category'}, {'Field': 'survey', 'Type': 'category'}, {'Field': 'telescope', 'Type': 'category'}, {'Field': 'updated', 'Type': 'datetime64'}]
________________________________________ TestNoirlabClass.test_core_hdu_fields _________________________________________

self = <astroquery.noirlab.tests.test_noirlab_remote.TestNoirlabClass object at 0x135f1df10>

    def test_core_hdu_fields(self):
        """List the available CORE HDU fields."""
        r = Noirlab(which='hdu').core_fields()
        actual = r
        print(f'DBG: test_core_hdu_fields={actual}')
        expected = exp.core_hdu_fields
>       assert actual == expected
E       AssertionError: assert [{'Field': 'b...: 'str'}, ...] == {'boundary': ... match'], ...}
E         Use -v to get the full diff

astroquery/noirlab/tests/test_noirlab_remote.py:134: AssertionError
------------------------------------------------- Captured stdout call -------------------------------------------------
DBG: test_core_hdu_fields=[{'Field': 'boundary', 'Type': 'str'}, {'Field': 'dec', 'Type': 'np.float64'}, {'Field': 'dec_range', 'Type': 'str'}, {'Field': 'fitsfile', 'Type': 'str'}, {'Field': 'fitsfile__archive_filename', 'Type': 'str'}, {'Field': 'fitsfile__caldat', 'Type': 'str'}, {'Field': 'fitsfile__date_obs_max', 'Type': 'str'}, {'Field': 'fitsfile__date_obs_min', 'Type': 'str'}, {'Field': 'fitsfile__dec_max', 'Type': 'str'}, {'Field': 'fitsfile__dec_min', 'Type': 'str'}, {'Field': 'fitsfile__depth', 'Type': 'str'}, {'Field': 'fitsfile__exposure', 'Type': 'str'}, {'Field': 'fitsfile__filesize', 'Type': 'str'}, {'Field': 'fitsfile__ifilter', 'Type': 'str'}, {'Field': 'fitsfile__instrument', 'Type': 'str'}, {'Field': 'fitsfile__md5sum', 'Type': 'str'}, {'Field': 'fitsfile__obs_mode', 'Type': 'str'}, {'Field': 'fitsfile__obs_type', 'Type': 'str'}, {'Field': 'fitsfile__original_filename', 'Type': 'str'}, {'Field': 'fitsfile__proc_type', 'Type': 'str'}, {'Field': 'fitsfile__prod_type', 'Type': 'str'}, {'Field': 'fitsfile__proposal', 'Type': 'str'}, {'Field': 'fitsfile__ra_max', 'Type': 'str'}, {'Field': 'fitsfile__ra_min', 'Type': 'str'}, {'Field': 'fitsfile__release_date', 'Type': 'str'}, {'Field': 'fitsfile__seeing', 'Type': 'str'}, {'Field': 'fitsfile__site', 'Type': 'str'}, {'Field': 'fitsfile__survey', 'Type': 'str'}, {'Field': 'fitsfile__telescope', 'Type': 'str'}, {'Field': 'fitsfile__updated', 'Type': 'str'}, {'Field': 'hdu_idx', 'Type': 'np.int64'}, {'Field': 'id', 'Type': 'str'}, {'Field': 'ra', 'Type': 'np.float64'}, {'Field': 'ra_range', 'Type': 'str'}, {'Field': 'updated', 'Type': 'datetime64'}]
__________________________________________ TestNoirlabClass.test_categoricals __________________________________________

self = <astroquery.noirlab.tests.test_noirlab_remote.TestNoirlabClass object at 0x135f47750>

    def test_categoricals(self):
        """List categories."""
        r = Noirlab().categoricals()
        actual = r
        print(f'DBG: test_categoricals={actual}')
        expected = exp.categoricals
>       assert actual == expected
E       AssertionError: assert {'instruments...1', ...], ...} == {'collections...d', ...], ...}
E         Omitting 7 identical items, use -vv to show
E         Right contains 1 more item:
E         {'collections': []}
E         Use -v to get the full diff

astroquery/noirlab/tests/test_noirlab_remote.py:169: AssertionError
------------------------------------------------- Captured stdout call -------------------------------------------------
DBG: test_categoricals={'instruments': ['(p)odi', '90prime', 'andicam', 'arcoiris', 'arcon', 'bench', 'ccd_imager', 'chiron', 'cosmos', 'decam', 'echelle', 'falmingos', 'flamingos', 'goodman', 'goodman spectrograph', 'gtcam', 'hdi', 'ice', 'ispi', 'kosmos', 'minimo/ice', 'mop/ice', 'mosaic', 'mosaic3', 'mosaic_1', 'mosaic_1_1', 'mosaic_2', 'newfirm', 'osiris', 'sami', 'soi', 'spartan', 'spartan ir camera', 'triplespec', 'wfc', 'whirc', 'wildfire', 'y4kcam'], 'obsmodes': [], 'proctypes': ['instcal', 'mastercal', 'nota', 'projected', 'raw', 'resampled', 'skysub', 'stacked'], 'prodtypes': ['dqmask', 'expmap', 'graphics (size)', 'image', 'image 2nd version 1', 'image1', 'nota', 'resampled', 'weight', 'wtmap'], 'sites': ['cp', 'ct', 'kp', 'lp'], 'surveys': [], 'telescopes': ['bok23m', 'ct09m', 'ct13m', 'ct15m', 'ct1m', 'ct4m', 'ctlab', 'kp09m', 'kp21m', 'kp35m', 'kp4m', 'kpcf', 'lp25m', 'soar', 'wiyn']}
============================================ 3 failed, 12 passed in 17.38s =============================================

Failed tests were due to new release of our server that happen to come after I submitted working tests. Fixed locally but awaiting guidance on how to best combine changes to PR

pothiers avatar Apr 23 '20 17:04 pothiers

@pothiers yes, let's rebase this and try to disentangle the histories. Ping me on slack if you need help

keflavich avatar Apr 29 '20 01:04 keflavich

@keflavich - git history-wise this is all right now

bsipocz avatar Apr 29 '20 02:04 bsipocz

CI has 2 test failing with astropy-dev due to this: https://github.com/astropy/astropy/issues/10227, thus it shouldn't be a blocker for this PR. However the async/sync issue should still be addressed before merging.

bsipocz avatar Apr 29 '20 03:04 bsipocz

Have added more "advanced search" documentation.

Thank you. What I had in mind is to have a bit more exact examples, driven by simple science cases to showcase the functionality of the module (not necessarily just the advanced search), otherwise I'm afraid the users won't know what the module is capable of.
This can be addressed in a follow-up later.

bsipocz avatar Apr 29 '20 03:04 bsipocz

I'm getting two failures when I run the remote tests, both the same:

AttributeError: 'Table' object has no attribute 'pformat_all'

ceb8 avatar Apr 29 '20 13:04 ceb8

The idea behind async_to_sync is to avoid duplicate code. In brief:

  • an _async method should return a requests response object
  • a _parse_result function should be defined that takes a response object and parses it into an astropy table
  • if async_to_sync is used, you should not need to define query_<blah>, you should only define query_<blah>_async.

In the current version of this code, async_to_sync has no effect because query_region_async and query_region are both defined explicitly. You don't need to use async_to_sync - it is, admittedly, a bit confusing. However, there is no reason to duplicate code in query_region and in query_region_async. I'll make an explicit code suggestion to illustrate this.

keflavich avatar May 18 '20 18:05 keflavich