separation_from stopped working for vector positions

[andreww5au]

Hi, in between versions 1.22 and 1.23, the separation_from() method stopped working between one position with vector components, and another position with single-valued components. It's a similar broadcast error to Issue #229 (although 229 remains fixed as of version 1.25).

Here's some example code:

----------------
import numpy
import skyfield.api as si
s = si.Star(ra=si.Angle(degrees=numpy.array([120.0,130.0])),dec=si.Angle(degrees=numpy.array([-30.0, -40.0])))
ts = si.load.timescale()
t = ts.utc(2019, 10, 14)
MWA_TOPO = si.Topos(longitude=(116, 40, 14.93), latitude=(-26, 42, 11.95), elevation_m=377.8)
PLANETS = si.load('de421.bsp')
S_MWAPOS = PLANETS['earth'] + MWA_TOPO
observer = S_MWAPOS.at(t)
pos = observer.observe(s)
s2 = si.Star(ra=si.Angle(degrees=123.0), dec=si.Angle(degrees=-35.0))
pos2=observer.observe(s2)
pos2.separation_from(pos).degrees
--------------------

For versions 1.22 and below, that code gives:
array([5.60288734, 7.46810989])

For versions 1.23 and above, it throws an exception:
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/andrew/mandcenv/lib/python3.8/site-packages/skyfield/positionlib.py", line 246, in separation_from
    return Angle(radians=angle_between(self.position.au,
  File "/home/andrew/mandcenv/lib/python3.8/site-packages/skyfield/functions.py", line 58, in angle_between
    a = u * length_of(v)
ValueError: operands could not be broadcast together with shapes (3,) (2,)

[brandon-rhodes]

Hi, in between versions 1.22 and 1.23, the separation_from() method stopped working between one position with vector components, and another position with single-valued components.

Indeed, alas! I had never tried that combination myself so there was no test in place to signal that, as the routine was enhanced, that the 1-to-many broadcast shape was broken.

I landed a fix three days ago:

#414

— but was waiting on the go-ahead from the person who reported the problem before doing a release. Is there any chance you could test for me, so I know if I've really fixed the problem for an end-user? You could install from master with:

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

If you can confirm that it’s fixed I’ll make an immediate release to get this out to you!

[andreww5au]

Thanks Brandon, yep, that works, all good now. Now that my code runs, I did notice another change since 1.16 (haven't pinned down exactly where yet) - casting a skyfield time object to a bool used to work (all times evaluated to True), but now it throws an exception:

import skyfield.api as si
ts = si.load.timescale()
t = ts.utc(2019, 10, 14)
not t
Traceback (most recent call last):
File "", line 1, in
File "/home/andrew/mandcenv/lib/python3.8/site-packages/skyfield/timelib.py", line 321, in len
return self.shape[0]
IndexError: tuple index out of range

The same thing happens if you call 'bool(t)' explicitly. It actually revealed a bug in my code (I should have used 'is None', as that's what I was actually testing for), so that worked out well too :-)

[brandon-rhodes]

Interesting, that __len__() method was added in 2015, so I am a bit at a loss as to why your code would suddenly have problems with it now. 🙂

I am glad to hear that separation_from() is now working for you, and I'll try to get the new version out today!

[andreww5au]

Sorry, you're right, casting a time to a bool was a change in my code, that I'd never tested.

Thanks!

[brandon-rhodes]

I have just released Skyfield 1.26, which includes this fix. Thanks again for letting me know!