TimeDelta and quantity input checking

[wafels]

A TypeError is raised when passing in a TimeDelta to a function decorated with the quantity input decorator and a time-like type annotation for the input.

The code below illustrates the issue. To me it seems natural to pass in a TimeDelta object as a way of expressing a duration, but perhaps I using TimeDelta incorrectly.

import astropy.units as u
from astropy.time import Time

@u.quantity_input
def f(t: u.s):
    print(t)

dt = Time('2018-10-09') - Time('2018-10-08')

f(dt)

[mhvk]

@wafels - we've tried to make TimeDelta interact with Quantity properly, but mostly in direct operations between the two, not quite with the purpose of TimeDelta acting quite directly as a higher precision Quantity. But maybe we can move this closer...

Looking at the actual code in units/decorators.py, the validation fails because a TimeDelta does not have a unit attribute. In principle, one could quite easily change this (and that is not a bad idea), but I worry this would cause other problems if somewhere inside the function you want to use the argument and count on it being a Quantity: a TimeDelta does not have all the attributes and methods that a Quantity has (e.g., one cannot meaningfully take a .view).

One possible solution would be to adjust the validation in the decorator to convert any non-Quantity argument to a Quantity, which means we would need to ensure that Quantity(td) would always work. (To change the validator like this may be a good idea regardless of this issue: it would, e.g., also convert Column instances with a unit to Quantity and thus avoid problems down the line with Column not doing anything with the unit in operations...)

Anyway, to do this, I think two steps are needed:

1. Allow u.Quantity(time_delta) to work; this is not difficult, see my quick trial at https://github.com/astropy/astropy/compare/master...mhvk:timedelta-more-like-quantity?expand=1 (this just adds a unit attribute to TimeDelta, as well as an __array__ method which allows np.array(time_delta) to work. Not obvious, though, that one wouldn't want to pass back in the units implied by the format!
2. Adjust the decorator to run u.Quantity on non-quantity instances. This is not as easy, as currently only validation is done, no conversion.

Note that both steps may be useful in themselves, although I must admit that I'm not 100% sure either is a good idea! E.g., for (2), we could also decide that the decorator should be stricter and simply insist that the input is a Quantity subclass...

cc @adrn, @Cadair (decorator) and @taldcroft (timedelta)

[Cadair]

Just my thoughts on 2), I am strongly against having u.quantity_input run u.Quantity on input, the original idea of the implementation was for it to duck type the input as much as possible (@adrn broke this with his physical types support) and I would like to hang on to it if possible. I would prefer the decorator to pass through a TimeDelta object directly if it were to have a .unit.

[mhvk]

@Cadair - I agree very much with the sentiment of duck-typing, but just to be sure, let me a bit more concrete about problems resulting from passing objects that have a unit attribute but do not behave like a Quantity: if you pass in a Column with, e.g., units of degree (e.g., do table['ra'], where table is a Table instance, not a QTable), then those will be checked by quantity_input and passed through (if you wanted angular units), but subsequently ignored in any operation:

In [8]: a = Column([30.], unit=u.degree)

In [9]: np.sin(a)
Out[9]: 
<Column dtype='float64' unit='deg' length=1>
-0.9880316240928618

In [10]: np.sin(u.Quantity(a))
Out[10]: <Quantity [0.5]>

Needless to say this behaviour is not great, but it was decided that we could not break Table by changing it (indeed, this is why we have QTable...). Still, users of the decorator could be forgiven for expecting that they get an object out that behaves properly like a Quantity... The same would hold for a TimeDelta - it can have the right unit, but it does not properly override __array_ufunc__ so functions that happily work on a quantity with time units may not work on the TimeDelta (in this respect, just adding an __array__ method is in fact asking for trouble...)