ENH: adding SSA method for IRSA by bsipocz · Pull Request #3076 · astropy/astroquery

bsipocz · 2024-08-01T00:21:23Z

Addig SSA access to spectral holdings. The query_ssa method is a very thin wrapper of pyvo, so while any issues recovered with it is appreciated, we may ultimately need to propagate those upstream rather than adding workaround for the PR.

closes #3061

keflavich · 2024-09-07T18:31:51Z

I'm testing this out now. See #3092 for related work.

First, the format is not the same as sia, which ideally we'd fix:

In [6]: crd = SkyCoord.from_name('Sgr B2')
   ...: sgrb2_q = Irsa.query_ssa(pos=(crd.ra, crd.dec, 0.01,) )

Traceback (most recent call last):
  File ~/mambaforge/envs/py310forge/lib/python3.10/site-packages/pyvo/dal/ssa.py:383 in pos
    ra, dec = pos
ValueError: too many values to unpack (expected 2)

During handling of the above exception, another exception occurred:
Traceback (most recent call last):
  Cell In[6], line 2
    sgrb2_q = Irsa.query_ssa(pos=(crd.ra, crd.dec, 0.01,) )
  File ~/repos/astroquery/astroquery/ipac/irsa/core.py:160 in query_ssa
    return self.ssa.search(pos=pos, diameter=diameter, band=band, time=time,
  File ~/mambaforge/envs/py310forge/lib/python3.10/site-packages/pyvo/dal/ssa.py:214 in search
    return self.create_query(
  File ~/mambaforge/envs/py310forge/lib/python3.10/site-packages/pyvo/dal/ssa.py:260 in create_query
    return SSAQuery(
  File ~/mambaforge/envs/py310forge/lib/python3.10/site-packages/pyvo/dal/ssa.py:351 in __init__
    self.pos = pos
  File ~/mambaforge/envs/py310forge/lib/python3.10/site-packages/pyvo/dal/ssa.py:385 in pos
    raise ValueError(
ValueError: Pos must be a sequence with exactly two values, expressing ra and dec in icrs degrees

ssa only wants a 2-tuple, not a 3-tuple

But it looks like right now, this is not yet functional (it is labeled draft, after all)

In [7]: crd = SkyCoord.from_name('Sgr B2')
   ...: sgrb2_q = Irsa.query_ssa(pos=(crd.ra, crd.dec) )
Traceback (most recent call last):
  Cell In[7], line 2
    sgrb2_q = Irsa.query_ssa(pos=(crd.ra, crd.dec) )
  File ~/repos/astroquery/astroquery/ipac/irsa/core.py:160 in query_ssa
    return self.ssa.search(pos=pos, diameter=diameter, band=band, time=time,
  File ~/mambaforge/envs/py310forge/lib/python3.10/site-packages/pyvo/dal/ssa.py:215 in search
    pos=pos, diameter=diameter, band=band, time=time, format=format, **keywords).execute()
  File ~/mambaforge/envs/py310forge/lib/python3.10/site-packages/pyvo/dal/ssa.py:578 in execute
    return SSAResults(self.execute_votable(), url=self.queryurl, session=self._session)
  File ~/mambaforge/envs/py310forge/lib/python3.10/site-packages/pyvo/dal/adhoc.py:111 in __init__
    super().__init__(votable, url=url, session=session)
  File ~/mambaforge/envs/py310forge/lib/python3.10/site-packages/pyvo/dal/query.py:322 in __init__
    raise DALQueryError(self._status[1], self._status[0], url)
DALQueryError: UsageFault: BAD_REQUEST: Unknown parameter: request

In [8]: crd = SkyCoord.from_name('Sgr B2')
   ...: sgrb2_q = Irsa.query_ssa(pos=crd)
Traceback (most recent call last):
  Cell In[8], line 2
    sgrb2_q = Irsa.query_ssa(pos=crd)
  File ~/repos/astroquery/astroquery/ipac/irsa/core.py:160 in query_ssa
    return self.ssa.search(pos=pos, diameter=diameter, band=band, time=time,
  File ~/mambaforge/envs/py310forge/lib/python3.10/site-packages/pyvo/dal/ssa.py:215 in search
    pos=pos, diameter=diameter, band=band, time=time, format=format, **keywords).execute()
  File ~/mambaforge/envs/py310forge/lib/python3.10/site-packages/pyvo/dal/ssa.py:578 in execute
    return SSAResults(self.execute_votable(), url=self.queryurl, session=self._session)
  File ~/mambaforge/envs/py310forge/lib/python3.10/site-packages/pyvo/dal/adhoc.py:111 in __init__
    super().__init__(votable, url=url, session=session)
  File ~/mambaforge/envs/py310forge/lib/python3.10/site-packages/pyvo/dal/query.py:322 in __init__
    raise DALQueryError(self._status[1], self._status[0], url)
DALQueryError: UsageFault: BAD_REQUEST: Unknown parameter: request

bsipocz · 2024-09-07T19:14:00Z

Yeap, whatever I have here is just a dump of the first draft that I came up with during some meeting or such by mostly just copying over sia, without cleaning up and making it ssa specific. Realistically this won't get done before the next quarter (and will need some server-side stuff, too, e.g. caching some queries)

codecov · 2025-02-14T18:49:08Z

Codecov Report

Attention: Patch coverage is 53.33333% with 7 lines in your changes missing coverage. Please review.

Project coverage is 69.07%. Comparing base (29ba772) to head (eecd8e1).
Report is 326 commits behind head on main.

Files with missing lines	Patch %	Lines
astroquery/ipac/irsa/core.py	50.00%	7 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3076      +/-   ##
==========================================
+ Coverage   68.31%   69.07%   +0.75%     
==========================================
  Files         231      232       +1     
  Lines       19199    19605     +406     
==========================================
+ Hits        13116    13542     +426     
+ Misses       6083     6063      -20

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

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

bsipocz · 2025-02-14T19:01:31Z

@keflavich - this is ready for review. There may be corners of uncovered API, and some missing parsing on the pyvo side, I've opened issues for a couple of those, but the basic examples should work here.

I'm also planning to make this more user friendly, e.g. adding the to_table() in the methods, but that will have to be systematic and do in e.g. the alma module, too.

keflavich

Minor suggestions & questions. Fine to merge, but it might be nice to make some small doc improvements.

keflavich · 2025-02-16T16:35:49Z

+   >>> arp220_spectra = Irsa.query_ssa(pos=coord).to_table()
+
+Without specifying the collection, the query returns results from multiple
+collections. For example this target has spectra from Sofia as well as from


SOFIA is an acronym, right?

Suggested change

collections. For example this target has spectra from Sofia as well as from

collections. For example this target has spectra from SOFIA as well as from

keflavich · 2025-02-16T16:36:51Z

+        pos : `~astropy.coordinates.SkyCoord` class or sequence of two floats
+            the position of the center of the circular search region.
+            assuming icrs decimal degrees if unit is not specified.
+        diameter : `~astropy.units.Quantity` class or scalar float


why diameter instead of radius? radius is more common across the rest of our API

inherited from SSA API, but I agree that I need to change this to be radius instead.

keflavich · 2025-02-16T16:38:00Z

+        format : str
+           the image format(s) of interest.  "all" indicates
+           all available formats; "graphic" indicates
+           graphical images (e.g. jpeg, png, gif; not FITS);
+           "metadata" indicates that no images should be
+           returned--only an empty table with complete metadata.


is ('all', 'graphic', 'metadata') a complete set of valid options? If so, it would be helpful to indicate that. If not, can we document what is allowed here or in the main docs?

I copied this from the pyvo docs, looking at the IRSA SSA metadata, these are the actual options:

<OPTION value="ALL"/> <OPTION value="COMPLIANT"/> <OPTION value="NATIVE"/> <OPTION value="VOTABLE"/> <OPTION value="FITS"/> <OPTION value="XML"/> <OPTION value="GRAPHIC"/> <OPTION value="METADATA"/>

However, some of these are known not to be working (e.g. metadata), and now as I test them some of them are not working as expected. (e.g. FITS).

So this may need an upstream fix (either at the server on in pyvo), and here, I may just remove this kwarg for now and hard-wire to return all results in this first iteration.

so, now I hardwired 'all', and opened a follow-up to address this properly. xref: #3234

keflavich · 2025-02-16T16:38:47Z

+        coord = SkyCoord.from_name("Eta Carina")
+        result = Irsa.query_ssa(pos=coord)
+        assert len(result) > 260
+        collections = set(unique(result.to_table(), keys='dataid_collection')['dataid_collection'])


is set(unique( redundant? I'd imagine so

sure 🤦‍♀️

bsipocz · 2025-02-27T23:58:15Z

I've addressed all the review issues here and opened follow-up for the format issues. So I'll go ahead with the merge so we can try and sort out if there is anything needed for the next release (in ~2 weeks time).

bsipocz added enhancement ipac.irsa labels Aug 1, 2024

bsipocz mentioned this pull request Aug 1, 2024

IRSA: adding SSA method #3061

Closed

bsipocz force-pushed the ENH_irsa_ssa branch from 78d9c10 to e6679a4 Compare August 1, 2024 00:24

bsipocz added this to the v0.4.8 milestone Dec 5, 2024

bsipocz modified the milestones: v0.4.8, v0.4.9 Jan 16, 2025

bsipocz removed this from the v0.4.9 milestone Jan 24, 2025

bsipocz added 4 commits February 13, 2025 18:06

Adding draft for IRSA's SSA method

4cb08f2

DOC: adding documentation for query_ssa

04d3007

Fix URL error

7c2d61e

Cleanup keywords, add mandatory ones for now

9e4a5a0

bsipocz force-pushed the ENH_irsa_ssa branch from e6679a4 to 9e4a5a0 Compare February 14, 2025 18:38

DOC: adding changelog

e784621

bsipocz added this to the v0.4.10 milestone Feb 14, 2025

TST: addig remote_data to docs example

151f791

TST: adding query_ssa test

900c4ad

bsipocz marked this pull request as ready for review February 14, 2025 18:57

bsipocz requested a review from keflavich February 14, 2025 19:01

keflavich reviewed Feb 16, 2025

View reviewed changes

Addressing review

2eeada5

bsipocz force-pushed the ENH_irsa_ssa branch from 7482617 to 2eeada5 Compare February 27, 2025 23:24

bsipocz mentioned this pull request Feb 27, 2025

ENH: Irsa.query_ssa to handle format keyword better #3234

Open

Fix style issues

eecd8e1

bsipocz merged commit 71b00bb into astropy:main Feb 28, 2025

bsipocz deleted the ENH_irsa_ssa branch February 28, 2025 00:00

	collections. For example this target has spectra from Sofia as well as from
	collections. For example this target has spectra from SOFIA as well as from

Uh oh!

Conversation

bsipocz commented Aug 1, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

keflavich commented Sep 7, 2024

Uh oh!

bsipocz commented Sep 7, 2024

Uh oh!

codecov Bot commented Feb 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

bsipocz commented Feb 14, 2025

Uh oh!

keflavich left a comment

Choose a reason for hiding this comment

Uh oh!

keflavich Feb 16, 2025

Choose a reason for hiding this comment

Uh oh!

keflavich Feb 16, 2025

Choose a reason for hiding this comment

Uh oh!

bsipocz Feb 19, 2025

Choose a reason for hiding this comment

Uh oh!

keflavich Feb 16, 2025

Choose a reason for hiding this comment

Uh oh!

bsipocz Feb 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bsipocz Feb 27, 2025

Choose a reason for hiding this comment

Uh oh!

keflavich Feb 16, 2025

Choose a reason for hiding this comment

Uh oh!

bsipocz Feb 20, 2025

Choose a reason for hiding this comment

Uh oh!

bsipocz commented Feb 27, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

bsipocz commented Aug 1, 2024 •

edited

Loading

codecov Bot commented Feb 14, 2025 •

edited

Loading

bsipocz Feb 20, 2025 •

edited

Loading