Access to NOIRLab Astro Data Archive

[weaverba137]

This PR closes #1701.

The initial commit simply restores the NOIRLab files to the state and content they were when #1701 was last worked on. Further work and testing will be needed to meet astroquery standards.

[weaverba137]

TO DO items identified (i.e. before even starting the review process):

• [x] Proofread noirlab/noirlab.rst and verify examples.
• [x] Remove telescope holdings from init file. These are certainly outdated. Link to the about page.
• [x] Check required acknowledgment(s).
• [x] Get remote tests working.
• [x] Add non-remote tests to increase coverage, e.g. monkeypatch.

[codecov[bot]]

Codecov Report

:x: Patch coverage is 96.84211% with 3 lines in your changes missing coverage. Please review. :white_check_mark: Project coverage is 70.63%. Comparing base (82cab62) to head (f643b79). :warning: Report is 145 commits behind head on main.

Files with missing lines	Patch %	Lines
astroquery/noirlab/core.py	96.59%	3 Missing :warning:

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3359      +/-   ##
==========================================
+ Coverage   70.07%   70.63%   +0.56%     
==========================================
  Files         232      234       +2     
  Lines       19893    20092     +199     
==========================================
+ Hits        13940    14192     +252     
+ Misses       5953     5900      -53     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

:rocket: New features to boost your workflow:

• :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[weaverba137]

Note to self: tox -e py310-test-online -- -P noirlab to do remote tests on only astroquery.noirlab.

[bsipocz]

You also need -R to run the remote tests

[weaverba137]

@keflavich @bsipocz this is ready for first-pass review.

I'm having some trouble with the doc build:

reading sources... [100%] xmatch/xmatch

/home/docs/checkouts/readthedocs.org/user_builds/astroquery/checkouts/3359/docs/noirlab/noirlab.rst:171: CRITICAL: Title level inconsistent:

astroquery.noirlab Package
-------------------------- [docutils]
/home/docs/checkouts/readthedocs.org/user_builds/astroquery/checkouts/3359/docs/noirlab/noirlab.rst:176: CRITICAL: Title level inconsistent:

Classes
^^^^^^^ [docutils]
looking for now-outdated files... none found

I've tried to set up the title levels similar to other packages, such as sdss.

I'm also looking for advice on whether further application of async_to_sync is needed.

[weaverba137]

PS, the .retrieve() method needs further testing. In the previous PR, this method actually returned a HDUList rather than a filename or file object. Could you please advise on how this should be handled in line with similar astroquery methods?

[bsipocz]

Thanks. I need to get back to the ESO review first, but put this on the list now, too.

[weaverba137]

@bsipocz, thank you, the suggestions all look good so far, but given my schedule it will be easier to address them all when you're done.

[bsipocz]

Yes, totally understandable. I really just submitted this half baked review as that is how far we got during the review tutorial during the summer school I was teaching at. I'll get back to the rest later and will write up an actually usable summary, as there are a couple of things that will need to be fixed while the rest is really just some potential follow-up topics.

And again, thanks for working on this, getting a working noirlab module will be superb!

[weaverba137]

Thank you, I was planning to pick this up next week anyway, so this is good timing.

[weaverba137]

Commit 64b84bf addresses some of the review comments. Others will be addressed soon.

There is an issue that needs to be investigated on the NOIRLab side though. Since I last tested in July, when metadata fields are requested in a query, e.g. md5sum, the field is returned, but a duplicate field with header file:md5sum is also returned. This behavior was unexpected both by me and the developer I mentioned this to.

[weaverba137]

Also, I have no clue why the noirlab.rst is causing a RtD error, because the headers are consistent with other files.

[bsipocz]

Also, I have no clue why the noirlab.rst is causing a RtD error, because the headers are consistent with other files.

That error rings a bell, and will be related to the generated API docs. I would recommend ignoring it for now and I'll have a closer look once this is ready to go in.

[weaverba137]

@bsipocz, I'm planning to resume work on this PR in the upcoming weeks, in anticipation of a new API version being deployed on the NOIRLab side.

One important question above is about function_name_async functions. I responded to a suggestion with further questions, which I still need answers for.

There is a less urgent question about whether sia_url should be an attribute/property. In NOIRLab's case there are two separate URLs so it kind of has to be a function.

Other open suggestions are things I just need to look up or otherwise implement.