sot / chandra_time

Convert between various time formats relevant to Chandra.
https://sot.github.io/Chandra.Time
BSD 3-Clause "New" or "Revised" License
4 stars 1 forks source link

Add datetime support #18

Closed jeanconn closed 9 years ago

jeanconn commented 9 years ago

I assume we'll get datetime support for free when/if Chandra.Time becomes an astropy.time wrapper, but in the interim, it would be great to get direct DateTime().datetime support for easy access to month and year. That seems to be what 99% of the mx.Datetime calls are doing.

I would guess this enhancement would be trivial, but I don't quite understand the postprocess/preprocess setups in Chandra.Time.

It looks like you presently get the right time now if you pass a datetime object in (is that supported or just a side-effect of the iso parsing?).

taldcroft commented 9 years ago

Is this inspired by removing mxDateTime from telem_archive? I spent a few minutes looking at this and I think it can be done easily enough with substrings of currently supported formats. Granted that is hacky.

In general I'm wary of the Python datetime since it doesn't know about leap seconds. What about an alternate patch of adding support for year and doy attributes, which is what you really want.

taldcroft commented 9 years ago

BTW, there is nothing stopping us from using astropy.time directly in any Chandra ops applications if it makes sense. The cxcsec format works and has been validated. I suppose we might want to give it one more round of detailed scrutiny. The only downside is not automatically recognizing numerical formats.

jeanconn commented 9 years ago

Is this inspired by removing mxDateTime from telem_archive?

Inspired by removing mxDateTime from telem_archive, perigee_health_plots, an old bit of mica, guide_stat_reports, and acq_stat_reports. And yes, they can all be fixed with substrings (though I was planning on explicit conversions to datetime as I both didn't know about the leapsec problem and now don't think it would be a problem. How does astropy.time use datetime and deal with the leapsec issue?).

I think year, month, day (of month), and doy are actually the things I "really want" (not just year, doy).

taldcroft commented 9 years ago

astropy doesn't try to cover up this weakness of datetime:

In [5]: t = Time('2015-06-30 23:59:60.5')

In [6]: t.datetime
ValueError: second must be in 0..59

So basically if you are using datetime you need to do some adhoc fix of the inputs to prevent an exception.

taldcroft commented 9 years ago

https://github.com/astropy/astropy/issues/3831

taldcroft commented 9 years ago

Similar PR for Chandra.Time in work now.

jeanconn commented 9 years ago

Thanks for the clarification on datetime and yes, it would be great to have these attributes in astropy.time and Chandra.Time without the need for adhoc fixes to datetime.

jeanconn commented 9 years ago

Also, this may be a silly question, but would your attributes be pinned to a time system?

taldcroft commented 9 years ago

Also, this may be a silly question, but would your attributes be pinned to a time system?

No that's a very good question. My simple "can do it in an hour" implementation is in UTC because that is easy. Do you have a need for these quantities in a different time system?

jeanconn commented 9 years ago

I don't need quantities in a different time system, I'd just need to know that it was UTC (and not local or dependent on the input time system or some such).

taldcroft commented 9 years ago

Closed by #19.