TimeDelta is subclass of Time

[danielk333]

replace all isinstance with type ==

[togry]

Why? I had thought the usual recommendation was the other way around. Demanding class equality means that you cannot extend (subclass) and use object of the new class instead. Why would this be preferable?

[danielk333]

Because the inheritance of behavior in astropy is not implemented for Time and TimeDelta in the standard fashion. For example:

>>> from astropy.time import TimeDelta, Time
>>> a = Time(54636, format='mjd')
>>> a.iso
'2008-06-19 00:00:00.000'
>>> b = TimeDelta(3.2, format='sec')
>>> b.iso
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/danielk/venvs/sorts/lib/python3.7/site-packages/astropy/time/core.py", line 1734, in __getattr__
    return self.__getattribute__(attr)
AttributeError: 'TimeDelta' object has no attribute 'iso'

Which would be expected since TimeDelta(Time), the subclassing was done to manage the scale not the format portion of the classes which is what is of concern when it is input to functions. E.g. to a space object if t=TimeDelta then the absolute time is epoch + t while if t=Time then the absolute time is t, this cannot be detected by isinstance(t, Time) since this will be True in both situations.

Does that make sense?

[danielk333]

The alternative would be something like isinstance(t, Time) and not type(t) == TimeDelta but unfourtunetly this would allow subclasses of TimeDelta trough as well as subclasses of Time

[togry]

def is_absolutetime(t):
    return isinstance(t, Time) and not isinstance(t, TimeDelta)

Just as an illustration, here is a subclass of Time which is an absolutetime, overriding the ymdhms attribute to return the instant of time's date according to the "Eternal September" measure of time.

from astropy import Time, TimeDelta

t0 = Time('1993-09-01T00:00:00')

class EternalSeptemberTime(Time):
    def _ymdhms(self):
        yy, mm, dd, HH, MM, SS = super().__getattr__('ymdhms')
        if self < t0:
            return yy, mm, dd, HH, MM, SS
        y0, m0, d0, H0, M0, S0 = t0.ymdhms
        days = (self - t0).datetime.days

        return y0, m0, days, HH, MM, SS

    def __getattr__(self, attr):
        if attr == 'ymdhms':
            return self._ymdhms()

        return super().__getattr__(attr)

a = EternalSeptemberTime(54636, format='mjd'))
print(a.ymdhms)    # prints "(1993, 9, 5405, 0, 0, 0.0)"

[danielk333]

True, so probably when checking input arguments we should do

if isinstance(t, Time) and not isinstance(t, TimeDelta):
    #we know its absolute time
elif isinstance(TimeDelta):
    #its relative time
else:
    #convert to astropy time

Good catch! Ill change the implementation (these are the brain-farts that happens when you try to debug without coffee haha)

[danielk333]

Fix pushed to my develop 5765529d45bbd3ba5a430dc59ccb13b8fca2458a , what do you think? @togry