search

shbhuk / barycorrpy

Python version of Barycorr
GNU General Public License v3.0
37 stars 6 forks source link

float JDUTC argument #13

Closed mzechmeister closed 6 years ago

mzechmeister commented 6 years ago

Currently, get_BC_vel requires an Astropy Time object for JDUTC.

I propose to allow here also simple floats, lists of floats, or numpy arrays. This is more convenient, since it does not require astropy import outside barycorrpy.

If you like and agree, I can include a draft in my pull request.

shbhuk commented 6 years ago

Hi @mzechmeister , the reason we went for this was - 1) To avoid any ambiguity over JD and MJD. 2) So that the user is aware of the time stamp scale (UTC, TDB, etc.)

Is there any way you think that the user input can be entirely unambiguous.

mzechmeister commented 6 years ago
  1. To avoid any ambiguity over JD and MJD.

Because of that the argument is called JDUTC and not MJDUTC.

  1. So that the user is aware of the time stamp scale (UTC, TDB, etc.)

Because of that the argument is called JDUTC and not JDUTC_TDB.

For me, this is pretty obvious. Any remaining doubts can be clarified in documentation and docstring. If somebody is still unsure, an Astropy Time object can be still provided.

astrowright commented 6 years ago

Since astropy is a dependency, I don't think using astropy date objects is an unnecessary complication. It also requires the user to think hard about time to use the module, which I see as a feature, not a bug. It also allows the user to specify the time in whatever format they have it, which is definitely a feature.

Complicated machinery that requires care should not necessarily be easy to use.

mzechmeister commented 6 years ago

I see your point. I don't what to remove this feature, but just suggested to offer an additional shortcut.

shbhuk commented 6 years ago

I think for now we shall leave it as such. In the future we might consider incorporating UT1 corrections along with the leap second corrections to make the code more precise. This would involve an overhaul of the time management routine, in which case we can include this as an added feature.

Thanks

shbhuk commented 6 years ago

Hi, We realized that we have already incorporated UT1 corrections to find the Earth Orientation parameters (nutation, precession) through the PINT routine that we are using. Hence, included the float JDUTC functionality now - 6c1f16e666405c47a4e95ff3b01aec998d023958 , d260c426be3cb91f73ebf738eadfdeeb54da7e3a