Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ Deprecations and Removals
Bug Fixes
---------


- Fix a bug in the space frame equinox processing.
Update of the Hipparcos catalogue (I/311/hip2) used in the viewer doc [710]
Comment on lines +20 to +21
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need for the docs mentions in the changelog, and fix PR number syntax

Suggested change
- Fix a bug in the space frame equinox processing.
Update of the Hipparcos catalogue (I/311/hip2) used in the viewer doc [710]
- Fix a bug in the space frame equinox processing. [#710]



1.8 (2025-11-13)
Expand Down
27 changes: 16 additions & 11 deletions docs/mivot/viewer.rst
Original file line number Diff line number Diff line change
Expand Up @@ -50,12 +50,13 @@ or a ``DALResults`` instance.
.. doctest-skip::

>>> import astropy.units as u
>>> from astropy.io.votable import parse
>>> from astropy.coordinates import SkyCoord
>>> from pyvo.dal.scs import SCSService
>>> from pyvo.utils.prototype import activate_features
>>> from pyvo.mivot.viewer.mivot_viewer import MivotViewer
>>>
>>> scs_srv = SCSService("https://vizier.cds.unistra.fr/viz-bin/conesearch/V1.5/I/239/hip_main")
>>> scs_srv = SCSService("https://vizier.cds.unistra.fr/viz-bin/conesearch/V1.5/I/311/hip2")
>>> m_viewer = MivotViewer(
... scs_srv.search(
... pos=SkyCoord(ra=52.26708 * u.degree, dec=59.94027 * u.degree,
Expand Down Expand Up @@ -95,13 +96,14 @@ and that the data rows can be interpreted as instances of the ``mango:EpochPosit
.. doctest-skip::

>>> print(mivot_instance.spaceSys.frame.spaceRefFrame.value)
ICRS
FK5

.. doctest-skip::

>>> while m_viewer.next_row_view():
>>> print(f"position: {mivot_instance.latitude.value} {mivot_instance.longitude.value}")
position: 59.94033461 52.26722684
position: 59.90665631 52.12106214
position: 59.94033468 52.26722736
....

.. important::
Expand All @@ -125,19 +127,22 @@ We can also provide a complete instance representation that includes all fields

>>> print(repr(globals_instance))
{
"dmtype": "coords:SpaceSys",
"dmid": "SpaceFrame_ICRS",
"frame": {
"dmtype": "coords:SpaceSys",
"dmid": "SpaceFrame_ICRS",
"frame": {
"dmrole": "coords:PhysicalCoordSys.frame",
"dmtype": "coords:SpaceFrame",
"spaceRefFrame": {
"dmtype": "ivoa:string",
"value": "ICRS"
}
}
"dmtype": "ivoa:string",
"value": "FK5"
},
"equinox": {
"dmtype": "coords:Epoch",
"value": "J2000"
}
}
}


As you can see from the previous examples, model leaves (class attributes) are complex types.
This is because they contain additional metadata as well as values:

Expand Down
10 changes: 7 additions & 3 deletions pyvo/mivot/features/sky_coord_builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -89,10 +89,14 @@ def _get_time_instance(self, hk_field, besselian=False):
-----
MappingError: if the Time instance cannot be built for some reason
"""
# Process complex type "mango:DateTime
# Process complex type "mango:DateTime"
if hk_field['dmtype'] == "mango:DateTime":
representation = hk_field['representation']['value']
timestamp = hk_field['dateTime']['value']
# Process complex type "coords:epoch" used for the space frame equinox
elif hk_field['dmtype'] == "coords:Epoch":
representation = 'yr' if "unit" not in hk_field else hk_field.get("unit")
timestamp = hk_field['value']
# Process simple attribute
else:
representation = hk_field.get("unit")
Expand All @@ -104,7 +108,8 @@ def _get_time_instance(self, hk_field, besselian=False):

time_instance = self. _build_time_instance(timestamp, representation, besselian)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With this typo, I suspect this line doesn't get test coverage

Suggested change
time_instance = self. _build_time_instance(timestamp, representation, besselian)
time_instance = self._build_time_instance(timestamp, representation, besselian)

if not time_instance:
raise MappingError(f"Cannot build a Time instance from {hk_field}")
mode = "besselian" if besselian else "julian"
raise MappingError(f"Cannot build a Time instance from {hk_field} ({mode} date)")

return time_instance

Expand Down Expand Up @@ -175,7 +180,6 @@ def _get_space_frame(self):
coo_sys = self._mivot_instance_dict["spaceSys"]["frame"]
equinox = None
frame = coo_sys["spaceRefFrame"]["value"].lower()

if frame == 'fk4':
self._map_coord_names = SkyCoordMapping.default_params
if "equinox" in coo_sys:
Expand Down
82 changes: 49 additions & 33 deletions pyvo/mivot/tests/test_sky_coord_builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,10 @@
import pytest
from copy import deepcopy
from astropy.utils.data import get_pkg_data_filename
from astropy import units as u
from pyvo.mivot.version_checker import check_astropy_version
from pyvo.mivot.viewer.mivot_instance import MivotInstance
from pyvo.mivot.features.sky_coord_builder import SkyCoordBuilder
from pyvo.mivot.utils.exceptions import NoMatchingDMTypeError
from pyvo.mivot.utils.exceptions import NoMatchingDMTypeError, MappingError
from pyvo.mivot.viewer.mivot_viewer import MivotViewer
from pyvo.utils import activate_features

Expand Down Expand Up @@ -118,9 +117,8 @@
"value": "FK5"
},
"equinox": {
"dmtype": "coords:SpaceFrame.equinox",
"dmtype": "coords:Epoch",
"value": "2012",
"unit": "yr",
}
}
},
Expand Down Expand Up @@ -176,14 +174,35 @@
"ref": None,
},
"equinox": {
"dmtype": "coords:SpaceFrame.equinox",
"dmtype": "coords:Epoch",
"value": "2012",
"unit": "yr",
},
},
}


def check_skycoo(scoo, ra, dec, distance, pm_ra_cosdec, pm_dec, obstime):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if there is a better way to compare the values to a SkyCoord object.

"""
Check the SkyCoord instance against the constant values given as parameters
"""
try:
assert scoo.ra.degree == pytest.approx(ra)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't noticed this before, but in general we don't really use pytest.approx() but prefer the functionality from numpy.testing. It's not really a big issue here when comparing scalars, but elsewhere the numpy versions may be more preferred.

assert scoo.dec.degree == pytest.approx(dec)
if distance:
assert scoo.distance.pc == pytest.approx(distance)
if pm_ra_cosdec:
assert scoo.pm_ra_cosdec.value == pytest.approx(pm_ra_cosdec)
if pm_dec:
assert scoo.pm_dec.value == pytest.approx(pm_dec)
except AttributeError:
assert scoo.galactic.l.degree == pytest.approx(ra)
assert scoo.galactic.b.degree == pytest.approx(dec)

if obstime:
assert str(scoo.obstime) == obstime


def test_no_matching_mapping():
"""
Test that a NoMatchingDMTypeError is raised not mapped on mango:EpochPosition
Expand All @@ -201,27 +220,26 @@ def test_vizier_output():
mivot_instance = MivotInstance(**vizier_dict)
scb = SkyCoordBuilder(mivot_instance)
scoo = scb.build_sky_coord()
assert (str(scoo).replace("\n", "").replace(" ", "")
== "<SkyCoord (ICRS): (ra, dec) in deg(52.26722684, 59.94033461) "
"(pm_ra_cosdec, pm_dec) in mas / yr(-0.82, -1.85)>")
scoo = mivot_instance.get_SkyCoord()
assert (str(scoo).replace("\n", "").replace(" ", "")
== "<SkyCoord (ICRS): (ra, dec) in deg(52.26722684, 59.94033461) "
"(pm_ra_cosdec, pm_dec) in mas / yr(-0.82, -1.85)>")
check_skycoo(scoo, 52.26722684, 59.94033461, None,
-0.82, -1.85,
None)

vizier_dict["spaceSys"]["frame"]["spaceRefFrame"]["value"] = "Galactic"
mivot_instance = MivotInstance(**vizier_dict)
scoo = mivot_instance.get_SkyCoord()
assert (str(scoo).replace("\n", "").replace(" ", "")
== "<SkyCoord (Galactic): (l, b) in deg(52.26722684, 59.94033461) "
"(pm_l_cosb, pm_b) in mas / yr(-0.82, -1.85)>")
check_skycoo(scoo, 52.26722684, 59.94033461, None,
-0.82, -1.85,
None)

vizier_dict["spaceSys"]["frame"]["spaceRefFrame"]["value"] = "QWERTY"
mivot_instance = MivotInstance(**vizier_dict)
scoo = mivot_instance.get_SkyCoord()
assert (str(scoo).replace("\n", "").replace(" ", "")
== "<SkyCoord (ICRS): (ra, dec) in deg(52.26722684, 59.94033461) "
"(pm_ra_cosdec, pm_dec) in mas / yr(-0.82, -1.85)>")
check_skycoo(scoo, 52.26722684, 59.94033461, None,
-0.82, -1.85,
"J1991.250")


@pytest.mark.skipif(not check_astropy_version(), reason="need astropy 6+")
Expand All @@ -232,19 +250,24 @@ def test_vizier_output_with_equinox_and_parallax():
mivot_instance = MivotInstance(**vizier_equin_dict)
scb = SkyCoordBuilder(mivot_instance)
scoo = scb.build_sky_coord()
assert (str(scoo).replace("\n", "").replace(" ", "")
== "<SkyCoord (FK5: equinox=J2012.000): (ra, dec, distance) in "
"(deg, deg, pc)(52.26722684, 59.94033461, 1666.66666667) "
"(pm_ra_cosdec, pm_dec) in mas / yr(-0.82, -1.85)>")
check_skycoo(scoo, 52.26722684, 59.94033461, 1666.66666667,
-0.82, -1.85,
"J1991.250")

mydict = deepcopy(vizier_equin_dict)
mydict["spaceSys"]["frame"]["spaceRefFrame"]["value"] = "FK4"
mivot_instance = MivotInstance(**mydict)
scoo = mivot_instance.get_SkyCoord()
assert (str(scoo).replace("\n", "").replace(" ", "")
== "<SkyCoord (FK4: equinox=B2012.000, obstime=B1991.250): (ra, dec, distance) in "
"(deg, deg, pc)(52.26722684, 59.94033461, 1666.66666667) "
"(pm_ra_cosdec, pm_dec) in mas / yr(-0.82, -1.85)>")
check_skycoo(scoo, 52.26722684, 59.94033461, 1666.66666667,
-0.82, -1.85,
"B1991.250")

mydict = deepcopy(vizier_equin_dict)
mydict["spaceSys"]["frame"]["spaceRefFrame"]["value"] = "FK4"
mydict["spaceSys"]["frame"]["equinox"]["value"] = "J2012"
with pytest.raises(MappingError, match=r".*besselian date.*"):
mivot_instance = MivotInstance(**mydict)
scoo = mivot_instance.get_SkyCoord()


@pytest.mark.skipif(not check_astropy_version(), reason="need astropy 6+")
Expand All @@ -257,16 +280,9 @@ def test_simad_cs_output():
scb = SkyCoordBuilder(mivot_instance)
scoo = scb.build_sky_coord()

assert scoo.ra.degree == pytest.approx(269.45207696)
assert scoo.dec.degree == pytest.approx(4.69336497)
assert scoo.distance.pc == pytest.approx(1.82823411)
x = scoo.pm_ra_cosdec.value
y = (-801.551 * u.mas/u.yr).value
assert x == pytest.approx(y)
x = scoo.pm_dec.value
y = (10362.394 * u.mas/u.yr).value
assert x == pytest.approx(y)
assert str(scoo.obstime) == "J2000.000"
check_skycoo(scoo, 269.45207696, 4.69336497, 1.82823411,
-801.551, 10362.394,
"J2000.000")


def test_time_representation():
Expand Down
Loading