Expose erfa constants from erfam.h and use them in Time #3137

mhvk · 2014-11-28T18:49:57Z

In Time, some of the constants from erfam.h are copied, since these are needed in TimeDelta, where no appropriate erfa routine can be used. If it is easy, it would be neat to get these directly, as in, say, erfa.ELG.

from time.core.py, l80ff:

# L_G = 1 - d(TT)/d(TCG) -> d(TT)/d(TCG) = 1-L_G
# d(TCG)/d(TT) = 1/(1-L_G) = 1 + (1-(1-L_G))/(1-L_G) = 1 + L_G/(1-L_G)
ERFA_ELG = 6.969290134e-10
# L_B = 1 - d(TDB)/d(TCB)
ERFA_ELB = 1.550519768e-8

mhvk · 2014-11-28T19:05:29Z

@jwoillez - In #3141 you mentioned commit jwoillez@d41c149 that exposed the constants in erfam.h. I now included this here and changed Time and EarthLocation to make use of them. Thanks!

mhvk · 2014-11-29T16:55:34Z

@taldcroft - could you have a quick look at the usage of constants from erfam.h in Time? I went perhaps a bit overboard in also using the MJD zero point and seconds per day from erfa, but I guess the benefit is that it makes Time futureproof even for the eventuality that the IAU decides to redefine MJD or the length of the day...

eteq · 2014-11-29T20:05:11Z

astropy/erfa/erfa.py

Just to be sure, the order these comes out is always the same as in the C source, right? If not, there's the possibility that expressions like this won't work if e.g. this comes bfeore DAS2R...

Correct. The constants are stored in a list, in the order they are extracted from erfam.h.

mhvk · 2014-11-29T20:49:28Z

@eteq - and conversely, for this PR, which makes only small changes to erfa.py, there is no real reason to wait for #3159, correct?

taldcroft · 2014-11-30T18:21:27Z

astropy/erfa/erfa.py

How hard would it be to include the constant docstring from erfam.h?

jwoillez · 2014-12-01T08:38:18Z

@mhvk - Above is a pull request addressing @taldcroft's suggestion to include constant docstrings from erfam.h.

mhvk · 2014-12-01T16:16:40Z

@jwoillez - thanks for the PR, lovely to have the documentation for the constants included!

taldcroft · 2014-12-01T17:04:13Z

👏 @jwoillez

mhvk · 2014-12-01T17:19:26Z

Just for heck's sake: I wondered if it is easy to expose the documentation in the constant's docstring. I tried the most naive way,

ELB = (1.550519768e-8)
"""L_B = 1 - d(TDB)/d(TCB), and TDB (s) at TAI 1977/1/1.0"""

(and ELB.__doc__ = ...) but this doesn't work, perhaps because float.__doc__ is not writable. So, it would need wrapping? Ping our python guru: @embray

taldcroft · 2014-12-01T19:36:07Z

http://stackoverflow.com/questions/20227051/how-to-document-a-module-constant-in-python

mhvk · 2014-12-01T21:34:11Z

@taldcroft - Great, so a very simple addition of a : would expose the constants in the sphinx documentation! Though even better of course would be to have it be in the docstring... A bit more of a search doesn't turn up anything useful, so it may be that this would require subclassing float and int, which is perhaps a bit much! Anyway, not really important; great to just have the constants available in the first place!

embray · 2014-12-02T15:31:12Z

In most of the rest of Astropy, where applicable, I've written

foo = 123
"""Docstring goes here."""

which Sphinx should pick up.

mhvk · 2014-12-02T18:18:18Z

@embray, @taldcroft - I was hoping for the even better case where help(erfa.ELB) would give the docstring (just like the help for functions). That seems a bit less easy and is arguably not quite worth it (but I'll think about it nevertheless...).

Anyway, for now, it may be best to follow the rest of astropy and use the doc-string after the constant instead of #:

embray · 2014-12-02T22:32:16Z

Yeah, there is no syntax built into Python for documenting individual objects (module-level or otherwise). The string under the variable assignment is just a convention Sphinx understands. Won't help for help(). I've thought a lot about how to design some kind of proxy object that would enable this, and I suspect others have thought about it too, but there aren't many good possibilities.

There have been some discussions taking place the last several weeks in Python-ideas and python-dev about customizing attribute access on modules which might help somewhat, but only somewhat (well, it could help immensely, but it depends on what form the final product ends up taking, if anything is even changed at all which it might not).

astrofrog · 2014-12-11T11:45:00Z

@mhvk - could you finish this up and label as 'ready for final review' once it's ready?

mhvk · 2014-12-11T15:13:18Z

@astrofrog - I think this is ready as is. With the #:, the comments should appear in the documentation and I think this is slightly clearer than the docstring below (though I can easily change that of course). But I was really waiting for a conclusion on the erfa stuff. E.g., it would probably be handier to rebase after #3173 goes in, if that is the one we converge to. @eteq, @embray - should I indeed wait for that, or is it fine for this one to go in first?

eteq · 2014-12-11T21:03:25Z

I would say wait for #3173, because astropy/astropy-helpers#112 is up, which should be what we need to get the auto-generation stuff running

mhvk · 2014-12-17T02:14:26Z

Now rebased after #3166 went in, so no longer adds a new erfa.py. Ready for final review...

taldcroft · 2014-12-17T03:45:13Z

👍

mhvk · 2014-12-17T20:24:48Z

This should only get in after #3173 is merged!

eteq · 2014-12-17T22:35:58Z

Alright, #3173 is in, so rebase when ready, @mhvk.

This may also conflict with #3217 due to earth.py, but it should be pretty easy to rebase because the changes in #3217 are just renaming from erfa_time names to erfa.

…tion

mhvk · 2014-12-18T02:54:07Z

OK, rebased and travis is happy. I think this is ready to go in. @eteq - are you ok with rebasing #3217 after this is in? As you write, either direction is easy, really. But this PR should help make SkyCoord more dimension-independent as well.

eteq · 2014-12-18T04:20:39Z

Sure, I'll just merge this now seeing as how it's ready.

Expose erfa constants from erfam.h and use them in Time

mhvk added Low Priority erfa time labels Nov 24, 2014

jwoillez added a commit to jwoillez/astropy that referenced this pull request Nov 28, 2014

Expose ERFA constants (fixes astropy#3137)

d41c149

jwoillez mentioned this pull request Nov 28, 2014

Split erfa wrapper between python and cython sections #3141

Merged

mhvk pushed a commit to mhvk/astropy that referenced this pull request Nov 28, 2014

Expose ERFA constants (fixes astropy#3137)

14a7ab8

mhvk changed the title ~~Wishlist: expose erfa constants~~ Expose erfa constants from erfam.h and use them in Time Nov 28, 2014

mhvk force-pushed the erfa-constants branch from 1679a83 to 583c0f5 Compare November 28, 2014 19:10

embray added the Affects-dev PRs and issues that do not impact an existing Astropy release label Nov 28, 2014

embray added this to the v1.0.0 milestone Nov 28, 2014

mhvk force-pushed the erfa-constants branch 2 times, most recently from ce050ed to 417beaa Compare November 28, 2014 21:54

eteq reviewed Nov 29, 2014
View reviewed changes

eteq mentioned this pull request Nov 29, 2014

Auto-generate erfa.pyx and erfa.py #3159

Closed

taldcroft reviewed Nov 30, 2014
View reviewed changes

astropy/erfa/erfa.py Outdated

Copy link

Member

taldcroft Nov 30, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How hard would it be to include the constant docstring from erfam.h?

jwoillez mentioned this pull request Dec 1, 2014

Add documentations to ERFA constants mhvk/astropy#14

Merged

mhvk force-pushed the erfa-constants branch from ee617f2 to 3977006 Compare December 1, 2014 16:15

mhvk force-pushed the erfa-constants branch from 3977006 to 7457ebd Compare December 2, 2014 15:15

eteq mentioned this pull request Dec 2, 2014

ERFA wrapper tweaks #3173

Merged

mhvk force-pushed the erfa-constants branch from 7457ebd to 4b07821 Compare December 17, 2014 02:11

mhvk pushed a commit to mhvk/astropy that referenced this pull request Dec 17, 2014

Expose ERFA constants (fixes astropy#3137)

10ad093

mhvk added the Ready-for-final-review label Dec 17, 2014

mhvk mentioned this pull request Dec 17, 2014

Add mad_std() and constants to convert Gaussian FWHM <-> sigma #3208

Merged

jwoillez and others added 3 commits December 17, 2014 19:33

Expose ERFA constants (fixes astropy#3137)

78be152

Use erfa constants from erfam.h exposed in erfa in Time and EarthLoca…

098e4d7

…tion

Add documentations to ERFA constants

4561746

mhvk force-pushed the erfa-constants branch from 4b07821 to 4561746 Compare December 18, 2014 01:14

eteq added a commit that referenced this pull request Dec 18, 2014

Merge pull request #3137 from mhvk/erfa-constants

d765308

Expose erfa constants from erfam.h and use them in Time

eteq merged commit d765308 into astropy:master Dec 18, 2014

mhvk deleted the erfa-constants branch December 18, 2014 13:23

embray removed the Ready-for-final-review label Jan 13, 2015

embray mentioned this pull request Jan 13, 2015

liberfa Python further simplification #3181

Closed

Uh oh!

Expose erfa constants from erfam.h and use them in Time #3137

Expose erfa constants from erfam.h and use them in Time #3137

Conversation

mhvk commented Nov 28, 2014

Uh oh!

mhvk commented Nov 28, 2014

Uh oh!

mhvk commented Nov 29, 2014

Uh oh!

eteq Nov 29, 2014

Choose a reason for hiding this comment

Uh oh!

jwoillez Dec 1, 2014

Choose a reason for hiding this comment

Uh oh!

mhvk commented Nov 29, 2014

Uh oh!

taldcroft Nov 30, 2014

Choose a reason for hiding this comment

Uh oh!

jwoillez commented Dec 1, 2014

Uh oh!

mhvk commented Dec 1, 2014

Uh oh!

taldcroft commented Dec 1, 2014

Uh oh!

mhvk commented Dec 1, 2014

Uh oh!

taldcroft commented Dec 1, 2014

Uh oh!

mhvk commented Dec 1, 2014

Uh oh!

embray commented Dec 2, 2014

Uh oh!

mhvk commented Dec 2, 2014

Uh oh!

embray commented Dec 2, 2014

Uh oh!

astrofrog commented Dec 11, 2014

Uh oh!

mhvk commented Dec 11, 2014

Uh oh!

eteq commented Dec 11, 2014

Uh oh!

mhvk commented Dec 17, 2014

Uh oh!

taldcroft commented Dec 17, 2014

Uh oh!

mhvk commented Dec 17, 2014

Uh oh!

eteq commented Dec 17, 2014

Uh oh!

mhvk commented Dec 18, 2014

Uh oh!

eteq commented Dec 18, 2014

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants