TypeError when indexing a `Time` object.

[jrs65]

Related to #450, if I create a Time directly using the tt argument of it's __init__ I can cause a very similar ` error.

# testsfbug.py
import numpy as np
from skyfield import api, timelib

ts = api.load.timescale()

t = timelib.Time(ts=ts, tt=np.arange(5))
print(t[0])

gives

$ python testsfbug.py
Traceback (most recent call last):
  File "testsfbug.py", line 7, in <module>
    print(t[0])
  File "/Users/richard/Projects/CHIME/pipeline/daily_rev01/venv/lib/python3.7/site-packages/skyfield/timelib.py", line 342, in __getitem__
    t = Time(self.ts, self.whole[index], self.tt_fraction[index])
TypeError: 'float' object is not subscriptable

The cause of this seems to be that tt gets set to self.whole as an array, whereas self.fraction gets set to the default 0.0 and so stays as a scalar. Presumably either __getitem__ needs to be able to deal with this, or fraction needs to get broadcast to an array of the same length in the __init__.

[brandon-rhodes]

Good point. To make the standard approach of instantiating a Time object through a Timescale more efficient, the Time object does no checks of its own that the arrays it is given agree in size, as that would be redundant since similar checks are already done in the Timescale. I'll update the Time object docs to make clear the requirement that the arrays agree.

Could you outline how you wound up in the situation of having to do a low-level instantiation of a Time yourself? Also, if the Skyfield docs themselves led you into that situation, let me know and I'll get them fixed to recommend using a Timescale method instead. Thanks!

[jrs65]

It's fairly obscure why I'm doing this, but here's a vague explanation.

For our telescope code base we primarily represent times as 64 bit float UNIX times (usually in numpy arrays), however, we use skyfield for most of our ephemeris calculations, and sometimes we also want to be able to deal with Python datetimes, and many routines will accept all of these and convert where necessary. We also want routines to be able to take scalar or vector inputs (lists, or numpy arrays). The way that's done is by writing code that assumes it gets a non-zero length array input, and a decorator wrapper deals with the scalar case for you by wrapping a scalar into a length-1 array and back again. As skyfield has it's own internal way of treating scalar and array quantities I need to do the operation of turning a scalar Time into a vector one, which in previous versions I could do successfully by simply doing:

t_array = timelib.Time(ts=t_scalar.ts, tt=np.array([t_scalar.tt]))

It seems that maybe a more "public" way of doing this would be to use, so I'll likely switch to that.

t_array = t_scalar.ts.tt_jd(np.array([t_scalar.tt]))

[brandon-rhodes]

@jrs65 — Thanks for the explanation! Yes, the documented tt_jd() approach should work with all recent versions of Skyfield.

I am still weighing how best to approach this problem, and have just removed the “documentation request” label on this issue on the suspicion that I might do more than just upgrade the Time docstring (to something like "don't instantiate this directly! seriously!" instead of the more gentle guideline it currently offers). There are two possible approaches to accepting that users will sometimes create Time objects themselves and making it safer:

1. I could, minimally, fix the case where tt_fraction is not provided. Instead of leaving it as always 0.0 in that case, I could instead make it an array the same size as tt, which I believe would fix the exception you have run into.
2. Or I could, with more expense, support all combinations of array-or-scalar and array-or-scalar for tt and tt_fraction. This would be more expensive but a more thoroughgoing fix and make direct instantiation of Time objects a much more thoroughly supported API.

My guess is that I will go with Option 1 and then improve the docs to instruct users, if they supply both a JD and fraction, to themselves make sure they have the same dimensions.

[brandon-rhodes]

In the commit above, I have armored the Time object so that manual instantiations that don't provide a tt_fraction should nevertheless behave sensibly. I have also gone ahead and solved a related problem: the case where a user provided a scalar JD but array fraction to one of the Timescale methods which, as the official constructors, I want to be safe in all circumstances.

Let me know if you see further exceptions related to this issue. To try the development version:

pip install https://github.com/skyfielders/python-skyfield/archive/master.zip