Request for consistent handling of time values between spacecraft frames and tte objects

[joshuarwood]

@BillCleveland-USRA @AdamGoldstein-USRA

Currently when working with poshist history information from the GBM mission a user can do:

poshist = GbmPosHist.open(pos_file)
frames = poshist.get_spacecraft_frame()

t = Time(val, format)
frame = frames.at(t)

where a Time object is used to retrieve a spacecraft frame. In this case, the user does not need knowledge of the underlying time format used in the mission. They only need to create a valid time object.

However, this is incompatible with the time format in tte files:

tte = GbmTte.open(tte_file)
frame = frames.at(tte.trigtime) <-- results in an error

because the tte class has no inherent knowledge of the underlying time format from the mission.

Would it be possible to require knowledge of the time format in the core tte class? Inherited classes from missions would then be required to set this value, similar to how GbmPosHist knows the underlying time format needed to generate spacecraft frames. Functions like splice_time could then allow for either float or time class input with something like:

def __init__(self, data, trigtime..., time_format):
    self. format = time_format (need to figure out how to handle class + format string)

def splice_time(self, tstart, tstop)
    if isinstance(tstart, Time):
          tstart = getattr(tstart, self.format)

this would allow users the ability to provide Time values instead of needing to know the internal mission time formats themselves. You could also have a separate splice_time function for a formatted input if you want to avoid slowing down the regular float method. The benefit here is that you avoid having to caste the underlying data to astropy time objects.

There is also a need to return a Time class value for tte.trigtime

What do you think? Should I prototype this for a pull request?

[BillCleveland-USRA]

I'm not a big fan of making all time values an instance of Time. Mainly because it adds considerable overhead even when the library or end user is only interested in working with the primitive floating point value (regardless of scale).

We could add a convention that states that the mission extension should consider integer/floating point values to represent seconds from that mission's epoch value, and define a function similar to:

def time_val(value: Union[int, float, Time, dt.datetime]) -> Time:
    if isinstance(value, Time):
        return value
    elif isinstance(value, dt.datetime):
        return Time(value, format='datetime')
    elif isinstance(value, (int, float)):
        return Time(value, format=mission_format)
   throw TypeError()

That way functions like at(...) can convert primitive values to time with a simple function call.

That said, I'm still open to the idea of keeping time values in Time objects but with an eye towards taking advantage of its array support for returning multiple time values.

[joshuarwood]

I completely agree with you. I also want to keep the internal values as floats, not Time objects. I may have been unclear with my earlier description.

What I was proposing is the ability to pass a Time object as an argument to something like splice_time() and have splice_time() convert the argument to an appropriate format while keeping all internal calculations as they are. It would essentially be a float casting step for non-float input. Do you think that would be too much overhead? If yes then how about a separate function such as splice_time_at() for use with Time object arguments?

The goal here is to provide a function that does the time manipulation for the user without the user needing to know that the internal format is different.

I suppose the tricker thing is returning trigtime, tstart, tstop values. What about methods in the same spirit as get_spacecraft_frames() such as

tte.get_trigtime()
lc.data.get_tstart() # for binned data
lc.data.get_tstop() # for binned data

?

[BillCleveland-USRA]

After discussing this more with @joshuarwood, I'm okay with standardizing the time parameters for functions expecting a Time object, and functions returning time as a Time object. The benefits from having a consistent and simple rule for handling time will offset any performance hits from using astropy Time.