Repository navigation
Add Earth Mass and Jupiter Mass as units - #3907
Conversation
|
👍 to have them as units |
There was a problem hiding this comment.
I think there's a glitch with the earth symbol here. It looks like it's uf728 (which is undefined), but you probably meant u1f728 (ASTRONOMICAL SYMBOL FOR VERDIGRIS). However, I'd recommend using u2295 (CIRCLED PLUS) instead, since I think it's a lot more widely available in various fonts.
There was a problem hiding this comment.
This is still showing incorrectly for me. Did you push the change?
|
Seems like a no-brainer 👍 |
|
Several tests appear to be failing due to the addition of new units (this includes, for example, a doctest for the units listing page). So please try to resolve those locally first. |
There was a problem hiding this comment.
The latex form should be r'M_{\rm Jup}'. Actually, should it be r'M_{\rm J}'? I think that is used much more commonly (even in the IAU definition in [1])
[1] http://asa.usno.navy.mil/static/files/2014/Astronomical_Constants_2014.pdf
(note this also defines the moon-to-earth mass ratio).
Also http://maia.usno.navy.mil/NSFA/NSFA_cbe.html#MSMJ2009
|
@embray I think the tests passed, there was just a timeout error on one. |
|
Just needs a changelog entry under v1.1 / New Features / units. You can use |
|
@keflavich - can you rebase today so we can include in 1.1? |
7a58af5 to
07e0a7c
Compare
|
@astrofrog - done. I had to do one nontrivial merge that might be worth double-checking: Also, I think the [skip ci] message may have caused the rebase not to undergo ci, which is bad. I'll rebase again and modify that message... |
07e0a7c to
fe2df20
Compare
|
@keflavich - there are some PEP8 failures, can you fix? Thanks! |
There was a problem hiding this comment.
The indentation is messed up here (there's a tab)
There was a problem hiding this comment.
thanks - my mergetool doesn't follow my space-only standards.
There was a problem hiding this comment.
Should be M_earth - might as well make it consistent with M_jup (but include [ci skip] for this)
[ci skip]
|
Thanks! |
Add Earth Mass and Jupiter Mass as units
I have had need to use Earth Mass and Jupiter Mass as units, not just
constants. One can easily do this per-session, but I see no reason not to
include them in astropy, since they're often quoted as units for brown dwarf
and exoplanet studies.
The per-session approach:
I'd like to do the same for a Lunar mass (which isn't in constants either), but
I can't find a reliable reference for the mass including an uncertainty. I
don't have Allen's Astrophysical Quantities - could someone with access to that
book put up the numbers?