Leap Second Timeout

[bjfultn]

• added a 5 second timeout that we control to avoid long and unpredictable timeouts coming from the USNO leap second server
• set leap_update=False after the first call to BCPy when looping over an array of JDs. There's no need to check the leap second file at each iteration of this loop, regardless of whether the leap_update flag was initially set to True or False
• some small PEP8 compliance changes near where I was working
• deleted the build directory since this was causing mass confusion when trying to pip install from source
• incremented to version 0.3.1

[shbhuk]

I agree with points 3 and 4. The reason for point 2 is that the leap second file only gets updated if the JDs being queried are more than 6 months or so, after the time the leap second file was last written. So it is possible to be querying a big array of JDs where the first half is safe, but the second half violates the trigger and needs the file to be updated. Thoughts?

[shbhuk]

Also, perhaps just make it a 10 second timeout?

[bjfultn]

I see, ok, then I guess we need to make a separate function that checks whether the leap second file needs to be updated before we get to the loop. There you can check against the max of the input time array. Right now it looks like that logic is in JDUTC_to_JDTDB

Sure, 10 s is fine, I wasn't sure how long we should expect that call to take when the server is up. Keep in mind this timeout is currently limiting the speed of this package for all users.

For NEID, with a 10 s timeout it will take ~15 min to do the barycentric correction on all ~90 orders.

[bjfultn]

We could also just force the leap_update no matter what JDs are in the array as long as the user didn't turn it off.

[shbhuk]

That is fair. The time to check if an update is needed should be inconsequential during regular operations. If leap_update=False then this discussion does not matter, and during regular operations with it True (that is USNO working fine), if there is an update required then there will be one call to the server which should work. After that update the next round of checks shouldn't take too long.

[shbhuk]

We could also just force the leap_update no matter what JDs are in the array as long as the user didn't turn it off.

What do you mean? Forcefully update every call?

[shbhuk]

Does this help understand?

[bjfultn]

Yeah, for every call of get_BC_vel where leap_update==True just check that file once, before the loop starts and grab a new one if necessary. We don't need to check whether the times in the array are contained within the existing leap second file (except to append your warnings). Just check if there is a new file on the server and grab it if there is.

The way its currently coded up, the easiest patch for this would be to run something like tai_utc,warning,error=leap_manage(utctime=utctime,fpath=fpath,leap_update=leap_update) before the loop starts where utctime is the maximum from the array. Then always turn leap_update=False after that since we just updated the file if needed.

[shbhuk]

Well, the way the file check works is that it needs to know the JDs that need to be converted. So far example, you could have a leap second file from 2015, and that would not trip the update if you're querying JDs from 2012 for example, or 2014 or so on.

Edit: I misunderstood what you meant. Yes, I think that makes sense, do you want to add that your PR?

[bjfultn]

Right, but I'm saying it doesn't need to know. In normal operations it is a quick operation. So regardless of whether the user is going to query times that need the update or not we can just check that file and update it if necessary before any time conversions are performed.

For example, in astropy it is done at the time of package import even if the user won't be doing anything with that file.

[shbhuk]

I like your suggestion of using the maximum JD and just checking (if the user allows) that and turning the rest off.

Edit: Would you like to include it in this PR? Also you're right in utc_tdb.py, it should be urllib2

[bjfultn]

OK, PR updated with that change.