-
-
Notifications
You must be signed in to change notification settings - Fork 400
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
WIP: NRAO Archive Query replacement #3015
base: main
Are you sure you want to change the base?
Conversation
Hello @keflavich! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:
Comment last updated at 2024-06-02 15:09:36 UTC |
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) |
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)
|
Codecov ReportAttention: Patch coverage is
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 ☔ View full report in Codecov by Sentry. |
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. |
A few things:
|
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 |
(additional commit message was just debug notes and should be ignored) add back tapsql stuff remove alma stuff from nrao add nrao obscore thing Adapt region queries to work with NRAO TAP service add tapsql.py for nrao, updates to data columns supported by NRAO TAP fix imports implemented data retrieval code flush add VLA handling fix the wait-until-done step flex fixes
# TODO: this needs to be implemented | ||
# """ | ||
# pass | ||
|
||
|
||
class NraoClass(BaseQuery): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe you still want to inherit this from BaseVOQuery
, so the session things passed through pyvo correctly.
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:
NRAO_FORM_KEYS
There's more than that, but I'm pushing this WIP so that @jjtobin can join in on this.