astropy / astropy

Astronomy and astrophysics core library
https://www.astropy.org
BSD 3-Clause "New" or "Revised" License
4.44k stars 1.78k forks source link

Better control over units for decimally formatted Angles #7456

Open astrofrog opened 6 years ago

astrofrog commented 6 years ago

Currently setting decimal=True in Angle.to_string results in values that don't have any units:

In [31]: a = Angle(3.1 * u.deg)

In [32]: a.to_string(unit='deg', decimal=True) 
Out[32]: '3.1'

In [33]: a.to_string(unit='mas', decimal=True) 
Out[33]: '1.116e+07'

Now let's say that I want to show the angle in LaTeX with the unit. I can counter-intuitively set decimal to False for mas:

In [35]: a.to_string(unit='mas', decimal=False, format='latex') 
Out[35]: '$1.116e+07\\mathrm{mas}$'

but if I do this for degrees, the output is (of course) sexagesimal:

In [36]: a.to_string(unit='deg', decimal=False, format='latex') 
Out[36]: '$3^\\circ06{}^\\prime00{}^{\\prime\\prime}$'

Using fields=1 doesn't help since it just truncates the output:

In [37]: a.to_string(unit='deg', decimal=False, format='latex', fields=1) 
Out[37]: '$3^\\circ$'

What I need is a way to get '$3^\\circ$'. I think that the correct API should maybe be something like:

a.to_string(unit='deg', decimal=True, format='latex', show_decimal_unit=True) 

or something similar. This is needed for WCSAxes (https://github.com/astropy/astropy/pull/7318/files) where for now I am just getting around this issue by using custom units derived from the angle units.

pllim commented 6 years ago

Ping @mhvk

mhvk commented 6 years ago

Can we just call this a bug and add the unit? Certainly, the docstring doesn't say anything about the unit being eaten...

neutrinoceros commented 10 months ago

@mhvk in https://github.com/astropy/astropy/pull/13933 you added a comment that seems a bit contradictory with your latest reply here: https://github.com/astropy/astropy/blob/9bfb777e317bf27801782e49efb45c53494905f6/astropy/coordinates/angles/core.py#L349

So should we still consider the present issue as a clear bug ?

mhvk commented 10 months ago

@neutrinoceros - Hmm, that was not too helpful a comment. I think that in #13933 I just wanted to be sure I broke no tests and added the comment because I was surprised... I'm still quite happy to call this a bug and fix it. Though really I would like what is done to be consistent with Quantity.to_string()...

neutrinoceros commented 10 months ago

I spent about 40min trying to fix this and I found that indeed, a lot of tests (~15) break when this function is changed, for various reasons: 1) tests that are actively checking that the unit is dropped (for instance astropy/coordinates/tests/test_angles.py::test_angle_formatting) 2) Other to_string implementation changing behaviour as a result (e.g. SkyCoord.to_string() in astropy/coordinates/tests/test_sky_coord.py::test_to_string) 3) Extra functionality where there's a user-exposed toggle for hiding/showing units with decimal formatting (e.g. astropy/visualization/wcsaxes/tests/test_formatter_locator.py::test_formatter_no_format_with_units)

From which I draw the following observations

For all these reasons I'll avoid sinking more time into this until we can reach a consensus, as I don't want to end up goal posting my way to a fix.

I also note that @astrofrog's original report isn't clear (at least to me) on the last point:

Using fields=1 doesn't help since it just truncates the output:

In [37]: a.to_string(unit='deg', decimal=False, format='latex', fields=1) 
Out[37]: '$3^\\circ$'

What I need is a way to get '$3^\\circ$'.

Well it seems like there is a way to get the (allegedly) desired result, so I'm confused. Was this a mistake in the report ?

astrofrog commented 10 months ago

@neutrinoceros - I think I meant I wanted a way to get '$3.1^\\circ$'

neutrinoceros commented 9 months ago
from astropy.coordinates import Angle
a = Angle(3.1, unit)
repr(a.to_string(unit="deg", decimal=True, format="latex"))

currently gives

'$3.1\\mathrm{{}^{\\circ}}$'

which is basically the same, I believe ?