ENH: adding SSA method for IRSA #3076

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 66.66667% with 4 lines in your changes missing coverage. Please review.

Project coverage is 68.30%. Comparing base (29ba772) to head (900c4ad).
Report is 8 commits behind head on main.

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

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3076      +/-   ##
==========================================
- Coverage   68.31%   68.30%   -0.01%     
==========================================
  Files         231      231              
  Lines       19199    19211      +12     
==========================================
+ Hits        13116    13123       +7     
- Misses       6083     6088       +5

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

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

docs/ipac/irsa/irsa.rst

+   >>> 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

astroquery/ipac/irsa/core.py

+        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

astroquery/ipac/irsa/core.py

+        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.

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

astroquery/ipac/irsa/tests/test_irsa_remote.py

+        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 added enhancement ipac.irsa labels Aug 1, 2024

bsipocz mentioned this pull request Aug 1, 2024

IRSA: adding SSA method #3061

Open

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ENH: adding SSA method for IRSA #3076

ENH: adding SSA method for IRSA #3076

bsipocz commented Aug 1, 2024 •

edited

Loading

keflavich commented Sep 7, 2024

bsipocz commented Sep 7, 2024

codecov bot commented Feb 14, 2025 •

edited

Loading

bsipocz commented Feb 14, 2025

keflavich left a comment

keflavich Feb 16, 2025

keflavich Feb 16, 2025

bsipocz Feb 19, 2025

keflavich Feb 16, 2025

bsipocz Feb 20, 2025 •

edited

Loading

keflavich Feb 16, 2025

bsipocz Feb 20, 2025

	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

ENH: adding SSA method for IRSA #3076

Are you sure you want to change the base?

ENH: adding SSA method for IRSA #3076

Conversation

bsipocz commented Aug 1, 2024 • edited Loading

keflavich commented Sep 7, 2024

bsipocz commented Sep 7, 2024

codecov bot commented Feb 14, 2025 • edited Loading

Codecov Report

bsipocz commented Feb 14, 2025

keflavich left a comment

Choose a reason for hiding this comment

keflavich Feb 16, 2025

Choose a reason for hiding this comment

keflavich Feb 16, 2025

Choose a reason for hiding this comment

bsipocz Feb 19, 2025

Choose a reason for hiding this comment

keflavich Feb 16, 2025

Choose a reason for hiding this comment

bsipocz Feb 20, 2025 • edited Loading

Choose a reason for hiding this comment

keflavich Feb 16, 2025

Choose a reason for hiding this comment

bsipocz Feb 20, 2025

Choose a reason for hiding this comment

bsipocz commented Aug 1, 2024 •

edited

Loading

codecov bot commented Feb 14, 2025 •

edited

Loading

bsipocz Feb 20, 2025 •

edited

Loading