astroquery icon indicating copy to clipboard operation
astroquery copied to clipboard

WIP: NRAO Archive Query replacement

Open keflavich opened this issue 1 year ago • 7 comments

The NRAO Archive API has changed completely, which forced us to remove the old one. We therefore asked for a new module.

This is a WIP based on the ALMA TAP query service (so far).

There are a few to-do items:

  • [ ] set up the correct parameters in NRAO_FORM_KEYS
  • [ ] test
  • [ ] hook up to SOLR-based data download windows (can't use TAP for this)

There's more than that, but I'm pushing this WIP so that @jjtobin can join in on this.

keflavich avatar May 29 '24 20:05 keflavich

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

Line 38:22: E128 continuation line under-indented for visual indent Line 39:22: E128 continuation line under-indented for visual indent Line 42:23: E231 missing whitespace after ',' Line 47:19: E241 multiple spaces after ',' Line 48:19: E241 multiple spaces after ',' Line 49:19: E241 multiple spaces after ',' Line 50:19: E241 multiple spaces after ',' Line 259:28: E127 continuation line over-indented for visual indent Line 260:28: E127 continuation line over-indented for visual indent Line 291:21: E127 continuation line over-indented for visual indent Line 339:5: E303 too many blank lines (2) Line 341:18: E124 closing bracket does not match visual indentation Line 354:9: E265 block comment should start with '# ' Line 354:121: E501 line too long (166 > 120 characters) Line 363:33: E124 closing bracket does not match visual indentation Line 371:31: E124 closing bracket does not match visual indentation Line 393:11: E121 continuation line under-indented for hanging indent Line 411:36: E124 closing bracket does not match visual indentation Line 422:30: E124 closing bracket does not match visual indentation Line 440:121: E501 line too long (168 > 120 characters) Line 454:1: E303 too many blank lines (3)

Line 43:1: E302 expected 2 blank lines, found 1 Line 146:13: W503 line break before binary operator Line 254:121: E501 line too long (145 > 120 characters) Line 259:1: E303 too many blank lines (3)

Comment last updated at 2024-06-02 15:09:36 UTC

pep8speaks avatar May 29 '24 20:05 pep8speaks

This basic example works:

from astroquery.nrao import core
query="SELECT * FROM tap_schema.obscore WHERE CONTAINS(POINT('ICRS',s_ra,s_dec),CIRCLE('ICRS', 290.9583, 14.1, 0.01))=1"
result = core.Nrao.query_tap(query)
print(result)

keflavich avatar May 29 '24 20:05 keflavich

I broke something though:

from astropy.coordinates import SkyCoord
from astropy import units as u, constants

crd = SkyCoord.from_name('w51')
result2 = core.Nrao.query_region_async(crd, 0.05*u.deg)
print(result2)
Traceback (most recent call last):
  File ~/Downloads/nrao_tap_test.py:14
    result2 = core.Nrao.query_region_async(crd, 0.05*u.deg)
  File ~/repos/astroquery/astroquery/nrao/core.py:220 in query_region_async
    return self.query_async(payload=payload, **kwargs)
  File ~/repos/astroquery/astroquery/nrao/core.py:262 in query_async
    result = self.query_tap(query, maxrec=maxrec)
  File ~/repos/astroquery/astroquery/nrao/core.py:189 in query_tap
    return self.tap.search(query, language='ADQL', maxrec=maxrec)
  File ~/repos/pyvo/pyvo/dal/tap.py:272 in run_sync
    **keywords).execute()
  File ~/repos/pyvo/pyvo/dal/tap.py:1106 in execute
    return TAPResults(self.execute_votable(), url=self.queryurl, session=self._session)
  File ~/repos/pyvo/pyvo/dal/adhoc.py:111 in __init__
    super().__init__(votable, url=url, session=session)
  File ~/repos/pyvo/pyvo/dal/query.py:322 in __init__
    raise DALQueryError(self._status[1], self._status[0], url)
DALQueryError: PSQLException: ERROR: operator does not exist: scircle && text
  Hint: No operator matches the given name and argument types. You might need to add explicit type casts.
  Position: 1270

keflavich avatar May 29 '24 21:05 keflavich

Codecov Report

Attention: Patch coverage is 45.34884% with 94 lines in your changes are missing coverage. Please review.

Project coverage is 66.93%. Comparing base (462c2da) to head (076df46). Report is 16 commits behind head on main.

:exclamation: Current head 076df46 differs from pull request most recent head 0ecb26d

Please upload reports for the commit 0ecb26d to get more accurate results.

Files Patch % Lines
astroquery/nrao/core.py 41.25% 94 Missing :warning:
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3015      +/-   ##
==========================================
- Coverage   67.35%   66.93%   -0.42%     
==========================================
  Files         236      233       -3     
  Lines       18320    18354      +34     
==========================================
- Hits        12339    12286      -53     
- Misses       5981     6068      +87     

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

codecov[bot] avatar May 29 '24 21:05 codecov[bot]

It looks like the query is trying to use intersects with s_region,which ALMA data has, but VLA does not. If we wanted that column, we'd need to have that calculated and stored in the database.

So, for now, I think we need NRAO queries to use the format of the working query that you showed above.

jjtobin avatar May 29 '24 23:05 jjtobin

I broke something though:

A few things:

  • there maybe corner cases with pyvo that may not work. If that's the case open bug reports upstream with minimal failing examples.
  • Do not bother with sync and async in a way that fits in the "old" astroquery setup. Most of the async stuff we have are not in fact real async, don't use the servers' async capabilities. Some discussion is here: https://github.com/astropy/astroquery/issues/2598, but bottom line, don't duplicate methods for new modules (and alma has them for historical reason at this point).

bsipocz avatar Jun 01 '24 22:06 bsipocz

And while this is a WIP PR, it would be nice to keep a clean history, maybe even prepend commit messages for the temporary/debugging ones, and the style/fixing ones, so a later squash will be easier.

Useful tips on good commit messages are here, eventually I hope it will propagate into our devdocs: https://numpy.org/devdocs/dev/development_workflow.html#writing-the-commit-message

bsipocz avatar Jun 01 '24 22:06 bsipocz