Repository navigation
Closed
Labels
Description
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
github-actions commentedon Dec 3, 2024
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].
eerovaher commentedon Dec 3, 2024
The reason you are getting the error with
astropy7.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 atastropy/astropy/io/votable/tree.py
Line 1678 in 24d3dd9
"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 inio.votable.cc @tboch @tomdonaldson @pllim
ManonMarchand commentedon Dec 4, 2024
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.
tomdonaldson commentedon Dec 4, 2024
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.
pllim commentedon Dec 4, 2024
pllim commentedon Dec 4, 2024
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"?pllim commentedon Dec 4, 2024
Would changing the code to write out vounit rather than cds format as Eero suggested above be acceptable?
tomdonaldson commentedon Dec 5, 2024
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.
pllim commentedon Dec 5, 2024
Anyone here willing to try out the suggestion as PR? 😅 🙏
marscode-agent commentedon Dec 11, 2024
pllim commentedon Dec 19, 2024
Maybe #17570 ? 🤞
Avoid certain versions of Astropy due to known bug astropy/astropy#17497