Skip to content

Cannot write votable with units equivalent to Jy/beam #17497

Closed

Description

@ErikOsinga

Description

Before astropy v7.0.0, I was able to write a table to 'votable' format which has units Jy/beam.

e.g. in astropy v6.1.4:

In [12]: a['col_flux_peak'].unit
Out[12]: Unit("mJy / beam")

In [13]: a.write('~/test.xml',format='votable')

However, as of astropy v7.0.0, this raises an error (full trace at bottom)

ValueError: Unit 'beam' not supported by the CDS standard.

Why has this behaviour changed? And can it be updated such that Jy/beam units are allowed again?

Thanks in advance,
Erik

In [11]: a['col_flux_peak'].unit
Out[11]: Unit("mJy / beam")

In [12]: a.write('~/test.xml',format='votable')
---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
Cell In[12], line 1
----> 1 a.write('~/test.xml',format='votable')

File /usr/local/lib/python3.12/site-packages/astropy/table/connect.py:130, in TableWrite.__call__(self, serialize_method, *args, **kwargs)
    128 instance = self._instance
    129 with serialize_method_as(instance, serialize_method):
--> 130     self.registry.write(instance, *args, **kwargs)

File /usr/local/lib/python3.12/site-packages/astropy/io/registry/core.py:386, in UnifiedOutputRegistry.write(self, data, format, *args, **kwargs)
    381     format = self._get_valid_format(
    382         "write", data.__class__, path, fileobj, args, kwargs
    383     )
    385 writer = self.get_writer(format, data.__class__)
--> 386 return writer(data, *args, **kwargs)

File /usr/local/lib/python3.12/site-packages/astropy/io/votable/connect.py:176, in write_table_votable(input, output, table_id, overwrite, tabledata_format)
    173 table_file = from_table(input, table_id=table_id)
    175 # Write out file
--> 176 table_file.to_xml(output, tabledata_format=tabledata_format)

File /usr/local/lib/python3.12/site-packages/astropy/io/votable/tree.py:4384, in VOTableFile.to_xml(self, fd, compressed, tabledata_format, _debug_python_based_parser, _astropy_version)
   4382 for element_set in element_sets:
   4383     for element in element_set:
-> 4384         element.to_xml(w, **kwargs)

File /usr/local/lib/python3.12/site-packages/astropy/io/votable/tree.py:3991, in Resource.to_xml(self, w, **kwargs)
   3988         elm.to_xml(w, **kwargs)
   3990 for elm in self.tables:
-> 3991     elm.to_xml(w, **kwargs)
   3993 for elm in self.resources:
   3994     if elm.type != "meta":

File /usr/local/lib/python3.12/site-packages/astropy/io/votable/tree.py:3282, in TableElement.to_xml(self, w, **kwargs)
   3280     for element_set in (self.fields, self.params, self.groups, self.links):
   3281         for element in element_set:
-> 3282             element.to_xml(w, **kwargs)
   3283 elif kwargs["version_1_2_or_later"]:
   3284     index = list(self._votable.iter_tables()).index(self)

File /usr/local/lib/python3.12/site-packages/astropy/io/votable/tree.py:1678, in Field.to_xml(self, w, **kwargs)
   1676 attrib = w.object_attrs(self, self._attr_list)
   1677 if "unit" in attrib:
-> 1678     attrib["unit"] = self.unit.to_string("cds")
   1679 with w.tag(self._element_name, attrib=attrib):
   1680     if self.description is not None:

File /usr/local/lib/python3.12/site-packages/astropy/units/core.py:762, in UnitBase.to_string(self, format, **kwargs)
    720 r"""Output the unit in the given format as a string.
    721 
    722 Parameters
   (...)
    759 s
    760 """
    761 f = unit_format.get_format(format)
--> 762 return f.to_string(self, **kwargs)

File /usr/local/lib/python3.12/site-packages/astropy/units/format/cds.py:303, in CDS.to_string(cls, unit, fraction)
    298 @classmethod
    299 def to_string(
    300     cls, unit: UnitBase, fraction: bool | Literal["inline"] = False
    301 ) -> str:
    302     # Remove units that aren't known to the format
--> 303     unit = cls._decompose_to_known_units(unit)
    305     if not unit.bases:
    306         if unit.scale == 1:

File /usr/local/lib/python3.12/site-packages/astropy/units/format/generic.py:640, in Generic._decompose_to_known_units(cls, unit)
    633 """
    634 Partially decomposes a unit so it is only composed of units that
    635 are "known" to a given format.
    636 """
    637 if isinstance(unit, core.CompositeUnit):
    638     return core.CompositeUnit(
    639         unit.scale,
--> 640         [cls._decompose_to_known_units(base) for base in unit.bases],
    641         unit.powers,
    642         _error_check=False,
    643     )
    644 if isinstance(unit, core.NamedUnit):
    645     try:

File /usr/local/lib/python3.12/site-packages/astropy/units/format/generic.py:646, in Generic._decompose_to_known_units(cls, unit)
    644 if isinstance(unit, core.NamedUnit):
    645     try:
--> 646         cls._get_unit_name(unit)
    647     except ValueError:
    648         if isinstance(unit, core.Unit):

File /usr/local/lib/python3.12/site-packages/astropy/units/format/generic.py:585, in Generic._get_unit_name(cls, unit)
    582 @classmethod
    583 def _get_unit_name(cls, unit: NamedUnit) -> str:
    584     name = unit._get_format_name(cls.name)
--> 585     cls._validate_unit(name)
    586     return name

File /usr/local/lib/python3.12/site-packages/astropy/units/format/generic.py:592, in Generic._validate_unit(cls, unit, detailed_exception)
    590 if unit not in cls._units:
    591     if detailed_exception:
--> 592         raise ValueError(
    593             f"Unit '{unit}' not supported by the {cls.__name__} standard. "
    594             + cls._did_you_mean_units(unit)
    595         )
    596     raise ValueError()
    597 if unit in cls._deprecated_units:

ValueError: Unit 'beam' not supported by the CDS standard. 

Expected behavior

Expect writing table to 'votable' format with units Jy/beam to work.

How to Reproduce

Take any table with units Jy/beam in any of the columns and try to write it to votable format.

from astropy.table import Table
import astropy.units as u
b = Table()
b['flux'] = [5]
b['flux'].unit = u.Jy/u.beam
b.write('~/test.xml',format='votable')

### Versions

```python
import astropy
try:
    astropy.system_info()
except AttributeError:
    import platform; print(platform.platform())
    import sys; print("Python", sys.version)
    import astropy; print("astropy", astropy.__version__)
    import numpy; print("Numpy", numpy.__version__)
    import erfa; print("pyerfa", erfa.__version__)
    try:
        import scipy
        print("Scipy", scipy.__version__)
    except ImportError:
        print("Scipy not installed")
    try:
        import matplotlib
        print("Matplotlib", matplotlib.__version__)
    except ImportError:
        print("Matplotlib not installed")
platform
--------
platform.platform() = 'Linux-4.18.0-553.27.1.el8_10.x86_64-x86_64-with-glibc2.36'
platform.version() = '#1 SMP Tue Nov 5 04:50:16 EST 2024'
platform.python_version() = '3.12.2'

packages
--------
astropy              7.0.0
numpy                1.26.4
scipy                1.14.1
matplotlib           3.9.3
pandas               2.2.3
pyerfa               2.0.1.5

Activity

  1. github-actions commented on Dec 3, 2024

    @github-actions
    Contributor

    Welcome to Astropy 👋 and thank you for your first issue!

    A project member will respond to you as soon as possible; in the meantime, please double-check the guidelines for submitting issues and make sure you've provided the requested details.

    GitHub issues in the Astropy repository are used to track bug reports and feature requests; If your issue poses a question about how to use Astropy, please instead raise your question in the Astropy Discourse user forum and close this issue.

    If you feel that this issue has not been responded to in a timely manner, please send a message directly to the development mailing list. If the issue is urgent or sensitive in nature (e.g., a security vulnerability) please send an e-mail directly to the private e-mail [email protected].

  2. eerovaher commented on Dec 3, 2024

    @eerovaher
    Member

    The reason you are getting the error with astropy 7.0 but not with previous versions is that the "cds" unit formatter now validates its output properly and "beam" simply isn't a valid unit in the "cds" format. That being said, it is not clear to me why is the code at

    attrib["unit"] = self.unit.to_string("cds")
    using the "cds" format at all. I would expect that by default the code would try to follow the latest VOTable standard, in which case it should be using the "vounit" format, which can handle "beam" just fine. So to me it looks like this might be a bug in io.votable.

    cc @tboch @tomdonaldson @pllim

  3. ManonMarchand commented on Dec 4, 2024

    @ManonMarchand
    Member

    A remnant of VOTable < 1.4 where the default unit was not vounits?
    Related to: #11032
    Would it still be possible to have the option to use cds units, like in reading mode? I fear this might impact a few things downstream if there is no way of using the cds units anymore in the writer.

  4. tomdonaldson commented on Dec 4, 2024

    @tomdonaldson
    Contributor

    A remnant of VOTable < 1.4 where the default unit was not vounits?
    Related to: #11032
    Would it still be possible to have the option to use cds units, like in reading mode? I fear this might impact a few things downstream if there is no way of using the cds units anymore in the writer.

    I suspect you are correct @ManonMarchand, and I agree that both options should be possible. On write (VOTable creation) I'm not sure if we should make the behavior depend on what version is being created.

    One thing that is very clear is that there should never be a traceback due to a perceived invalid unit either on read or write. Nothing more serious than a warning should happen.

  5. pllim commented on Dec 4, 2024

    @pllim
  6. pllim commented on Dec 4, 2024

    @pllim
    Member

    Sorry, I didn't read this carefully at first. The error does come on write. Do we want people to write out "invalid" unit? What is the vounit equivalent of "beam"?

  7. pllim commented on Dec 4, 2024

    @pllim
    Member

    Would changing the code to write out vounit rather than cds format as Eero suggested above be acceptable?

  8. tomdonaldson commented on Dec 5, 2024

    @tomdonaldson
    Contributor

    This is messy due to the differences in the standard from version to version. In VOTable 1.5 the language was clarified (softened) to say that units "SHOULD conform to the VOUnits specification" (SHOULD meaning "doesn't absolutely have to"), so I think our best strategy is flexibility with no more than warnings being emitted for non-recommended units. Within the realm of being flexible, there are a number of possibilities with something like the below seeming OK to me, but definitely open for discussion:

    On write, for VOTable versions >= 1.4 we could default to using the VOUnit syntax, while retaining the previous default to CDS format for prior versions. In both cases, failure to create a valid unit string should result in a warning rather than a traceback. We could consider providing an API option to select which conversion to use, but creators could do the translation themselves since output of arbitrary unit strings should be supported to preserve some semblance of round-tripping and custom provider behavior.

  9. pllim commented on Dec 5, 2024

    @pllim
    Member

    Anyone here willing to try out the suggestion as PR? 😅 🙏

  10. marscode-agent commented on Dec 11, 2024

    @marscode-agent
  11. pllim commented on Dec 19, 2024

    @pllim
    Member

    Maybe #17570 ? 🤞

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions