Unit roundtrips do not always work

Description

Not sure whether this is an issue, however it would probably nice to have it fixed:

Expected behavior

I would expect that any roundtrip for unicode format works. i.e. for all units unit:

format='unicode'
assert u.Unit(unit.to_string(format=format), format=format) == unit

should work. I would expect the same for the other formatters; at least for generic and fits.

Actual behavior

For Angstrom, it works when not giving unicode as format in the u.Unit() constructor, but not when this is given:

>>> u.Unit(u.Angstrom.to_string(format='unicode')) == u.Angstrom
True
>>> u.Unit(u.Angstrom.to_string(format='unicode'), format='unicode') == u.Angstrom
[…]
ValueError: 'Å' did not parse as unicode unit: Can not parse Unicode If this […]

The second exception comes for any unit; unicode seems not be supported as parsing format.

For the solar mass, the first one does not work as well:

>>>  u.Unit(u.solMass.to_string(format='unicode')) == u.solMass
[…]
ValueError: 'M⊙' did not parse as unit: At col 0, M is not a valid unit. Did you […]

I would assume that

the "unicode" parse format is identical to the "generic" format (as we usually use unicode in strings)
Unicode output is also parseable, as the generic input

System Details

The tests were done with Astropy 4.2

@mhvk

I guess one could include the unicode stuff into the affected def_unit calls, but the bulk of the work is to smoke out all the affected units and add them in... Maybe @mhvk can advise.

Finding them is simple:

import astropy.units as u
import inspect

format='unicode'

print(f'|  Type        |  Generic     |  Unicode     | ')
print(f'| ------------ | ------------ | ------------ | ')

units = set(unit for name, unit in inspect.getmembers(u)
            if isinstance(unit, u.Unit)
            and not isinstance(unit, u.core.PrefixUnit))

for unit in units:
    formatted_name = unit.to_string(format=format)
    try:
        res = u.Unit(formatted_name) == unit
    except ValueError:
        res = False
    if not res:
        print(f'| {unit.physical_type:12s} | {unit.to_string():12s} | {formatted_name:12s} | ')

This shows all unique but the prefixed units that raise the error:

Type	Generic	Unicode
mass	M_e	Mₑ
power	solLum	L⊙
length	jupiterRad	R♃
mass	M_p	Mₚ
length	earthRad	R⊕
mass	earthMass	M⊕
angle	arcsec	″
volume	l	ℓ
mass	solMass	M⊙
mass	jupiterMass	M♃
angle	hourangle	ʰ
energy	Ry	R∞
angle	arcmin	′
length	solRad	R⊙

Looks not overwhelming to me. Maybe I will have a look myself, if this is agreed to be useful..

@mhvk

this is agreed to be useful

I don't see how it can hurt, though personally, I don't try to create a unit with unicode symbol. Let's see what @mhvk say first.

I am still a bit unsure how it works there. For example, the unit "Ohm" is defined here:

astropy/astropy/units/si.py

Lines 180 to 182 in 6387507

    
           def_unit((['Ohm', 'ohm'], ['Ohm']), V * A ** -1, namespace=_ns, prefixes=True, 
        
                    doc="Ohm: electrical resistance", 
        
                    format={'latex': r'\Omega', 'unicode': 'Ω'})

This works well, i.e. u.Unit(u.Ohm.to_string(format='unicode')) == u.Ohm

The definition of the liter is quite similar:

astropy/astropy/units/si.py

Lines 45 to 47 in 6387507

    
           def_unit((['l', 'L'], ['liter']), 1000 * cm ** 3.0, namespace=_ns, prefixes=True, 
        
                    format={'latex': r'\mathcal{l}', 'unicode': 'ℓ'}, 
        
                    doc="liter: metric unit of volume")

but it raises the exception.

I was thinking more like this:

diff --git a/astropy/units/si.py b/astropy/units/si.py
index 1e8f96e83..875691f31 100644
--- a/astropy/units/si.py
+++ b/astropy/units/si.py
@@ -32,7 +32,7 @@ def_unit(['micron'], um, namespace=_ns,
          doc="micron: alias for micrometer (um)",
          format={'latex': r'\mu m', 'unicode': '\N{MICRO SIGN}m'})
 
-def_unit(['Angstrom', 'AA', 'angstrom'], 0.1 * nm, namespace=_ns,
+def_unit(['Angstrom', 'AA', 'angstrom', 'Å'], 0.1 * nm, namespace=_ns
,
          doc="ångström: 10 ** -10 m",
          prefixes=[(['m', 'milli'], ['milli', 'm'], 1.e-3)],
          format={'latex': r'\mathring{A}', 'unicode': 'Å',

>>> u.Unit(u.Angstrom.to_string(format='unicode')) == u.Angstrom
True

Okay, sorry, what I did was this, not really what you asked for. Yeah, I am not sure how to get what you want...

>>> u.Å
Unit("Angstrom")

@maxnoe

Good suggestion! I think there are two separate issues here, one that the generic parser's unicode support for unit names is incomplete, and the other that there is no unicode parser, but only a unicode output format.

For the former, @maxnoe implemented quite a bit in #9348. As you can see, the reason you get some success is that the PR special-cases Ohm and Angstrom in

astropy/astropy/units/format/generic.py

Lines 452 to 458 in d6412f2

    
           if not _is_ascii(s): 
        
               if s[0] == '\N{MICRO SIGN}': 
        
                   s = 'u' + s[1:] 
        
               if s[-1] == '\N{GREEK CAPITAL LETTER OMEGA}': 
        
                   s = s[:-1] + 'Ohm' 
        
               elif s[-1] == '\N{LATIN CAPITAL LETTER A WITH RING ABOVE}': 
        
                   s = s[:-1] + 'Angstrom'

I think we could add to that to solve the remaining ones - or perhaps be a bit cleverer and specifically compare with for possible unicode definitions of units.

For allowing format='unicode' in parsing, we need to change the present state, which is that 'unicode' is only an output format class (like 'latex'). I first thought it might be as simple as letting unicode format class inherit from the generic format class, but then realized it is trickier: e.g., composite units are typeset differently, so for proper round-tripping, the parser would need to be adjusted to enable parsing things like:

In [3]: u.Jy.decompose().to_string(format='unicode')
Out[3]: '        kg\n1×10⁻²⁶ ──\n        s²'

In [4]: print(u.Jy.decompose().to_string(format='unicode'))
        kg
1×10⁻²⁶ ──
        s²

Do-able, of course, but perhaps not as worth it as just allowing the unit names to round-trip through generic...

The division in unicode is probably really too complicated to parse -- and I am also not sure if everybody expects this. For example, when building a table like the one above, it would destroy the table structure.
I would also not care too much about the unicode input specification -- however the error messages are a bit misleading then. This error message suggests that one can use unicode:

>>> u.Unit('Ω', format='foo')
[…]
ValueError: Unknown format 'foo'.  Valid formatter names are: [base, generic, unscaled, cds, console, fits, latex, latex_inline, ogip, unicode, vounit]

but then it also does not work.
For the acceptance of unicode characters at input: Maybe the simplest way would be to add the Unicodes as aliases directly to the def_unit() definitions?

Agreed, we should make clear that format='unicode' is not supported for parsing.

The one thing against just adding unicode to def_unit() is that then the symbol does become available as u.Ω as well - there was a specific decision in #9348 not to do that. That said, I guess one could keep them in the registry, separate from what is in the module.

@mhvk

@mhvk If I remember correctly, we said we could have a look again when we are python >= 3.7, which we are now, right?

I think it was for module __getattr__, like this:

import astropy.units as u

def __getattr__(name):
    try:
        return u.Unit(name)
    except:
        raise AttributeError(f'Module {__name__} has no attribute {name}')

enables this:


In [1]: import test

In [2]: test.µΩ
Out[2]: Unit("uOhm")

@maxnoe

OK, I made a fix to the parsing issue, see #11827.

For the parsing with 'unicode', it needs a better error message indeed, which I think is easier to deal with as a separate issue, so I raised #11828.

@maxnoe - indeed, you are right, we are now free to try to implement the module __getattr__ idea!

Navigation Menu

Repository navigation

Unit roundtrips do not always work #11826

Description

Expected behavior

Actual behavior

System Details

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

	def_unit((['Ohm', 'ohm'], ['Ohm']), V * A ** -1, namespace=_ns, prefixes=True,
	doc="Ohm: electrical resistance",
	format={'latex': r'\Omega', 'unicode': 'Ω'})

	def_unit((['l', 'L'], ['liter']), 1000 * cm ** 3.0, namespace=_ns, prefixes=True,
	format={'latex': r'\mathcal{l}', 'unicode': 'ℓ'},
	doc="liter: metric unit of volume")

	if not _is_ascii(s):
	if s[0] == '\N{MICRO SIGN}':
	s = 'u' + s[1:]
	if s[-1] == '\N{GREEK CAPITAL LETTER OMEGA}':
	s = s[:-1] + 'Ohm'
	elif s[-1] == '\N{LATIN CAPITAL LETTER A WITH RING ABOVE}':
	s = s[:-1] + 'Angstrom'

Unit roundtrips do not always work #11826

Description

Description

Expected behavior

Actual behavior

System Details

Activity

pllim commented on Jun 9, 2021

olebole commented on Jun 9, 2021

pllim commented on Jun 9, 2021

olebole commented on Jun 9, 2021

pllim commented on Jun 9, 2021

pllim commented on Jun 9, 2021

pllim commented on Jun 9, 2021

mhvk commented on Jun 9, 2021

olebole commented on Jun 9, 2021

mhvk commented on Jun 9, 2021

maxnoe commented on Jun 9, 2021

maxnoe commented on Jun 9, 2021

mhvk commented on Jun 9, 2021

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions