yt-project / unyt

Handle, manipulate, and convert data with units in Python
https://unyt.readthedocs.io
BSD 3-Clause "New" or "Revised" License
364 stars 48 forks source link

Types of solar units are confusing #87

Open ngoldbaum opened 5 years ago

ngoldbaum commented 5 years ago

This is weird:

In [1]: import unyt                                                             
In [2]: type(unyt.msun)                                                         
Out[2]: unyt.array.unyt_quantity

In [3]: type(unyt.Msun)                                                         
Out[3]: unyt.array.unyt_quantity

In [4]: type(unyt.unit_symbols.Msun)                                            
Out[4]: unyt.unit_object.Unit

In [5]: unyt.unit_symbols.msun                                                  
---------------------------------------------------------------------------
AttributeError                            Traceback (most recent call last)
<ipython-input-5-52dd1f84adcb> in <module>
----> 1 unyt.unit_symbols.msun

AttributeError: module 'unyt.unit_symbols' has no attribute 'msun'

In [6]: unyt.physical_constants.msun                                            
Out[6]: unyt_quantity(1.98841586e+30, 'kg')

In [7]: unyt.physical_constants.Msun                                            
Out[7]: unyt_quantity(1.98841586e+30, 'kg')

In [8]: unyt.unit_symbols.Msun                                                  
Out[8]: Msun

It was also confusing in the real world.

I'd like to get rid of unyt.physical_constants.Msun and unyt.physical_constants.msun and add a unyt.unit_symbols.msun but I think that might break code...

ngoldbaum commented 5 years ago

ping @jborrow

jzuhone commented 5 years ago

I'd prefer unyt.unit_symbols.Msun.

Maybe we need a separate module within unit for solar system masses / properties as unyt_quantity objects?

e.g.

from unyt.solar_system import Msun, Earth

JBorrow commented 4 years ago

There's also Solar_Mass which in some ways addresses this issue. It would be nice to define unyt.msun as 1 * unyt.Solar_Mass (or as an alias for it, actually).

P.S. There is also solar_mass which is inconsistent with Solar_Mass:

In [1]: unyt.solar_mass                                                                                                                                                             
Out[1]: unyt_quantity(1.98841586e+30, 'kg')

In [2]: unyt.Solar_Mass                                                                                                                                                             
Out[2]: Msun