astropy / astropy/pyvo

BUG: vector SkyCoords are not supported

Open
#409 6 comments 0 reactions 0 assignees View on GitHub
bug component: SIA
Dominant language
Python
Stars
88
Forks
64
Avg merge
38m
Merged PRs (30d)
1

Description

I was running into the issue of SIAv2 is not working with scalar coordinates (https://github.com/astropy/pyvo/issues/305), so I made them a vector SkyCoord, which is working even less, e.g. `_validate_pos()` in `dal.params` wrongly assumes ra and dec from it. IMO the whole position validation should be rewritten and make more reliant on astropy.coordinates as most of the logic is already been done there.

```
>>> seip.search(pos=SkyCoord([151.1, 151.1], [2.0, 2.0], unit="deg"), size=0.0)
File ~/munka/devel/pyvo/pyvo/dal/sia2.py:196, in SIAService.search(self, pos, band, time, pol, field_of_view, spatial_resolution, spectral_resolving_power, exptime, timeres, publisher_did, facility, collection, instrument, data_type, calib_level, target_name, res_format, maxrec, session, **kwargs)
181 def search(self, pos=None, band=None, time=None, pol=None,
182 field_of_view=None, spatial_resolution=None,
183 spectral_resolving_power=None, exptime=None,
(...)
186 target_name=None, res_format=None, maxrec=None, session=None,
187 **kwargs):
188 """
189 Performs a SIAv2 search against a SIAv2 service
190
(...)
194
195 """
--> 196 return SIAQuery(self.query_ep, pos=pos, band=band,
197 time=time, pol=pol,
198 field_of_view=field_of_view,
199 spatial_resolution=spatial_resolution,
200 spectral_resolving_power=spectral_resolving_power,
201 exptime=exptime, timeres=timeres,
202 publisher_did=publisher_did,
203 facility=facility, collection=collection,
204 instrument=instrument, data_type=data_type,
205 calib_level=calib_level, target_name=target_name,
206 res_format=res_format, maxrec=maxrec,
207 session=session, **kwargs).execute()

File ~/munka/devel/pyvo/pyvo/dal/sia2.py:269, in SIAQuery.__init__(self, url, pos, band, time, pol, field_of_view, spatial_resolution, spectral_resolving_power, exptime, timeres, publisher_did, facility, collection, instrument, data_type, calib_level, target_name, res_format, maxrec, session, **kwargs)
266 super().__init__(url, session=session)
268 for pp in _tolist(pos):
--> 269 self.pos.add(pp)
271 for bb in _tolist(band):
272 self.band.add(bb)

File ~/munka/devel/pyvo/pyvo/dal/params.py:258, in AbstractDalQueryParam.add(self, item)
257 def add(self, item):
--> 258 if item in self:
259 return
260 self._data.append(item)

File ~/munka/devel/pyvo/pyvo/dal/params.py:279, in AbstractDalQueryParam.__contains__(self, item)
277 def __contains__(self, item):
278 # check dal format for duplications since the quantities are known
--> 279 return self.get_dal_format(item) in self.dal

File ~/munka/devel/pyvo/pyvo/dal/params.py:303, in PosQueryParam.get_dal_format(self, val)
298 def get_dal_format(self, val):
299 """
300 formats the tuple values into a string to be sent to the service
301 entries in values are either quantities or assumed to be degrees
302 """
--> 303 self._validate_pos(val)
304 if len(val) == 3:
305 shape = 'CIRCLE'

File ~/munka/devel/pyvo/pyvo/dal/params.py:355, in PosQueryParam._validate_pos(self, pos)
353 self._validate_dec(m)
354 else:
--> 355 self._validate_ra(m)

File ~/munka/devel/pyvo/pyvo/dal/params.py:359, in PosQueryParam._validate_ra(self, ra)
357 def _validate_ra(self, ra):
358 if not isinstance(ra, Quantity):
--> 359 ra = ra * u.deg
360 if ra.to(u.deg).value < 0 or ra.to(u.deg).value > 360.0:
361 raise ValueError('Invalid ra: {}'.format(ra))

TypeError: unsupported operand type(s) for *: 'SkyCoord' and 'Unit'
```

Contributor guide

No contributing guide indexed for this repository

Research direction

Start in pyvo/dal/params.py at PosQueryParam._validate_pos() and _validate_ra(), then trace how SIAQuery in pyvo/dal/sia2.py adds position values. Reproduce the vector SkyCoord example from the issue and determine how position validation should rely on astropy.coordinates while preserving scalar SIAv2 behavior; done means vector SkyCoord positions are accepted without the shown TypeError.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
api, backend-api-design
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.