Error in __len__ for Time object

[JoshPaterson]

I created a time object and tested its length like this:

len(ts.utc(2018, 3, 23))

Because time.tt is a float64 object, self.shape is (). Then when len() calls self.shape[0] this error is raised:

Traceback (most recent call last):

  File "<ipython-input-5-1e372996a002>", line 1, in <module>
    len(ts.utc(2018, 3, 23))

  File "C:\ProgramData\Anaconda3\lib\site-packages\skyfield\timelib.py", line 218, in __len__
    return self.shape[0]

IndexError: tuple index out of range

A possible fix would be to change line 218 to this:

return self.tt.size

[brandon-rhodes]

My guess is that this would make the API inconsistent with the underlying logic of NumPy; let's see if I can work out why.

First — though this is the more crazy / unlikely of the two objections I can think of — the .size of a multi-dimensional array isn't the length of what you'd iterate over if you tried iterating across the array. It is, rather, the total number of elements across all dimensions:

import numpy as np
from skyfield.api import load

ts = load.timescale()
a = np.array([[3, 4, 5], [6, 7, 8]])
print(len(ts.tt(jd=a)))

This prints out 6 even though the user would only see length 2 if they actually iterated across the array.

But, maybe users never use n-dimensional time arrays, so, as I said, that might be a bit far-fetched.

The other problem is that, at least given how NumPy thinks about the world, floating point numbers in fact do not have length 1, they don't have a length at all. For NumPy, scalars by definition have no length. It explicitly doesn't consider a scalar to be an array of length 1 — as some other numerics systems do — but it thinks of them as having no length at all. (Unlike, say, Python strings, where if you break them into their littlest pieces you get strings of length 1.)

But I note that

print(len(np.float(3.3)))

gives

Traceback (most recent call last):
  File "tmp23.py", line 9, in <module>
    print(len(np.float(3.3)))
TypeError: object of type 'float' has no len()

instead of the inscrutable IndexError that Skyfield throws in this case. I would happily merge code that, if a time is a scalar value, raises a ValueError with a message like "scalar time objects have no length".

[JoshPaterson]

Thank you for taking the time to explain this. I now have a better understanding of the intentions behind unit objects in Skyfield, and also of numpy arrays in general. This explanation, along with your explanation in #151 makes great sense, and I agree that the current behavior is correct. I will put adding a more descriptive error message onto my todo list and in the meantime, if you want to close this issue, I see no reason not to.

[brandon-rhodes]

Okay, I'll close it — thanks for your patience as I work out how Skyfield should work, given the bigger ecosystem of which it's a part!