Give lat/lon array a str() that doesn’t raise ValueError

[wgwz]

Minimal code to repro:

iss = find_satellite("ISS (ZARYA)")
ts = load.timescale()
geocentric = iss.at(ts.now())
sample_size = 100
total_mins = 200 # approximately 3 and a half hours
interval = int(total_mins / sample_size) # minutes per sample
from skyfield.api import utc
times = {"past": [], "future": []}
total_time = interval
for i in range(int(sample_size / 2)):
    times["past"].append(datetime.utcnow().replace(tzinfo=utc) - timedelta(minutes=total_time))
    times["future"].append(datetime.utcnow().replace(tzinfo=utc) + timedelta(minutes=total_time))
    total_time += interval
past = iss.at(ts.from_datetimes(times["past"]))
future = iss.at(ts.from_datetimes(times["future"]))
breakpoint()

At that breakpoint you can see the errors by:

(Pdb) wgs84.subpoint(future)
*** ValueError: The truth value of an array with more than one element is ambiguous. Use a.any() or a.all()
(Pdb) wgs84.subpoint(past)
*** ValueError: The truth value of an array with more than one element is ambiguous. Use a.any() or a.all()

I was expecting to get back a list or multiple subpoint objects.

I just realized this is just a warning message and that the code does work as intended. So perhaps this is just a vanity thing to clean up or maybe it's something I've done wrong.

Proof it works as intended:

(Pdb) future_points = wgs84.subpoint(future)
(Pdb) future_points.latitude.degrees
array([ 40.02567957,  35.19123787,  29.92500048,  24.34328505,
        18.53587237,  12.57340305,   6.51363887,   0.40644326,
        -5.7021849 , -11.76657634, -17.73789652, -23.56023409,
       -29.16582637, -34.46906677, -39.3593364 , -43.69388955,
       -47.29473674, -49.95777065, -51.48454313, -51.73715061,
       -50.6908114 , -48.44458085, -45.18084694, -41.10678884,
       -36.41257692, -31.25460376, -25.75510598, -20.00841863,
       -14.08837606,  -8.05480355,  -1.95873081,   4.15341527,
        10.23620968,  16.24176508,  22.11594664,  27.79380953,
        33.19385636,  38.21100379,  42.70909598,  46.51613832,
        49.42958409,  51.24250449,  51.79526684,  51.03345195,
        49.03122698,  45.95993831,  42.02867312,  37.43616958,
        32.34866274,  26.896893  ])

Perhaps it's just tired eyes, but that message raised a red flag for me. I realize this may be an issue there's not much to do on. But I thought I'd share anyways.

Greatly enjoying this library. Thanks for your work on it :-)

[brandon-rhodes]

Alas, I don't get that warning on my particular NumPy version that's installed. Could you try putting this at the top of your script?

import numpy as np
np.seterr(all='raise')

Then re-run and it should give you a whole exception that you can paste here so we can learn where the error is. Thanks!

[wgwz]

Huh, oddly I'm only seeing this message when using pdb and inspecting either of these variables:

past_points = wgs84.subpoint(past)
future_points = wgs84.subpoint(future)
breakpoint()

here's the pdb session:

(Pdb) past_points
*** ValueError: The truth value of an array with more than one element is ambiguous. Use a.any() or a.all()
(Pdb) future_points
*** ValueError: The truth value of an array with more than one element is ambiguous. Use a.any() or a.all()

when i run the full script it, that error doesn't get mentioned. also i tried those lines you shared and that didn't seem to make a difference in the issue getting raised to the level of the script.

i should mention that this is code in a flask view function/controller:

  @app.route("/isspath")
  def isspath():
      iss = find_satellite("ISS (ZARYA)")
      ts = load.timescale()
      geocentric = iss.at(ts.now())
      sample_size = 100
      total_mins = 100 # approximately 3 and a half hours
      interval = int(total_mins / sample_size) # minutes per sample
      times = {"past": [], "future": []}
      total_time = interval
      for i in range(int(sample_size / 2)):
          times["past"].append(datetime.utcnow().replace(tzinfo=utc) - timedelta(minutes=total_time))
          times["future"].append(datetime.utcnow().replace(tzinfo=utc) + timedelta(minutes=total_time))
          total_time += interval
      past = iss.at(ts.from_datetimes(times["past"]))
      future = iss.at(ts.from_datetimes(times["future"]))
      past_points = wgs84.subpoint(past)
      future_points = wgs84.subpoint(future)
      past_res = list(zip(past_points.longitude.degrees, past_points.latitude.degrees))
      future_res = list(zip(future_points.longitude.degrees, future_points.latitude.degrees))
      return jsonify({"past": past_res, "future": future_res})

hmm.. i'll see if google fu for something like "numpy pdb valueerror" turns up any hints.

FYI, i'm on the latest numpy (1.19.4)

[brandon-rhodes]

Could the issue be that pdb tries looking at the resulting object, like by generating its str() or repr(), and it's that action which sets the warning off? You might try putting those calls into your script to check, and otherwise try investigating what pdb does to objects at your breakpoints.

[wgwz]

indeed! adding those calls to str or repr, both of these do get the script to raise the error now.

here's an example of some of the traceback for repr:

  File "/home/kyle/code/twilio/space-station-alerts/app/main.py", line 279, in isspath
    repr(past_points)
  File "/home/kyle/code/twilio/space-station-alerts/venv/lib/python3.8/site-packages/skyfield/vectorlib.py", line 30, in __repr__
    return '<{0} {1}>'.format(type(self).__name__, str(self))
  File "/home/kyle/code/twilio/space-station-alerts/venv/lib/python3.8/site-packages/skyfield/vectorlib.py", line 34, in __str__
    return self.target_name
  File "/home/kyle/code/twilio/space-station-alerts/venv/lib/python3.8/site-packages/skyfield/descriptorlib.py", line 12, in __get__
    value = self.method(instance)
  File "/home/kyle/code/twilio/space-station-alerts/venv/lib/python3.8/site-packages/skyfield/toposlib.py", line 73, in target_name
    e = ' elevation {0:.0f} m'.format(m) if m else ''
ValueError: The truth value of an array with more than one element is ambiguous. Use a.any() or a.all()

similarly for str:

  File "/home/kyle/code/twilio/space-station-alerts/app/main.py", line 278, in isspath                                                                                                                                                                                            
    str(past_points)                                                                                                                                                                                                                                                              
  File "/home/kyle/code/twilio/space-station-alerts/venv/lib/python3.8/site-packages/skyfield/vectorlib.py", line 34, in __str__                                                                                                                                                  
    return self.target_name                                                                                                                                                                                                                                                       
  File "/home/kyle/code/twilio/space-station-alerts/venv/lib/python3.8/site-packages/skyfield/descriptorlib.py", line 12, in __get__                                                                                                                                              
    value = self.method(instance)                                                                                                                                                                                                                                                 
  File "/home/kyle/code/twilio/space-station-alerts/venv/lib/python3.8/site-packages/skyfield/toposlib.py", line 73, in target_name                                                                                                                                               
    e = ' elevation {0:.0f} m'.format(m) if m else '' 
ValueError: The truth value of an array with more than one element is ambiguous. Use a.any() or a.all()

[brandon-rhodes]

Thanks for investigating further! I have edited the title to help me keep up with the specific fix needed here. I guess before the next release I should design how the string representation of a multiple-topographic-location array should show what it is. I wonder if it should print all the coordinates or just say "I represent n locations" and let the user poke inside if they want to see the actual latitudes and elevations etc or maybe instead leave it to NumPy to do any reasonable truncation (in case a user tries printing a location with, say, a thousand latitudes).

[wgwz]

No problem! Thanks for your help. I like the idea of having something "I represent n locations" but the latter option sounds good to me as well, I'm pretty easy going with these things.

Taking a look at what the single location str and repr's return:

(Pdb) str(past_point)
'WGS84 latitude 17deg 10\' 29.8" N longitude 106deg 24\' 49.0" E elevation 419876 m'
(Pdb) repr(past_point)
'<GeographicPosition WGS84 latitude 17deg 10\' 29.8" N longitude 106deg 24\' 49.0" E elevation 419876 m>'

I do really like this, it is helpful for someone debugging their code. It gives very quick feedback. You could easily go to google just from the output above to validate the result (if possible).

Here's an idea that could be nice ergonomically, currently this part of the API works like this:

(Pdb) repr(past_point)
<GeographicPosition WGS84 latitude 17deg 10' 29.8" N longitude 106deg 24' 49.0" E elevation 419876 m>
(Pdb) repr(future_points)
*** ValueError: The truth value of an array with more than one element is ambiguous. Use a.any() or a.all()
(Pdb) future_points[0]
*** TypeError: 'GeographicPosition' object is not subscriptable

What if the future_points was a new container class (not sure if this is the right term) called GeographicPositions? Then the above terminal session might look like:

(Pdb) future_points
<GeographicPositions I represent n locations>
(Pdb) future_points[0]
<GeographicPosition WGS84 latitude 17deg 10' 29.8" N longitude 106deg 24' 49.0" E elevation 419876 m>

You would be able to subscript in addition to getting a clean repr, and str. But I realize this very well may a lot of work and I am probably missing some important tradeoffs. I just wanted to share the idea :-)

[brandon-rhodes]

Thanks for the ideas! I'll mull them over and hopefully have my thoughts straight by sometime next week; I'll comment here at that point.

[brandon-rhodes]

I have updated this issue title since at its root, I think, is the fact that Skyfield has never handled lat/lon position arrays before (unless there's a spot in the docs and tests where they're used, that I've forgotten about?), and possibly many things need to change to offer them full support.

And thus I have also added the label “feature request” so that as I look at my dashboard I can keep track of “existing things that are broken” vs “new things we want to support”!

And thus this issue gets drawn into the larger discussion that is taking place over on Pull Requests #526 and #532.

My guess is that it’s only once the discussion there has arrived at a consistent approach towards arrays (which will probably take some time; as you'll note, I'm probably only making about one comment a week there, given my other commitments) that we’ll return here and be able to apply that approach to giving geographic positions and their str()'s.

So, thanks for your patience!

[wgwz]

FWIW, lat/lon position arrays do seem to "just work". I.e. this code works like a charm:

    iss = find_satellite("ISS (ZARYA)")
    ts = load.timescale()
    geocentric = iss.at(ts.now())
    sample_size = 100
    total_mins = 100  # approximately 3 and a half hours
    interval = int(total_mins / sample_size)  # minutes per sample
    times = {"past": [], "future": []}
    total_time = interval
    for i in range(int(sample_size / 2)):
        times["past"].append(
            datetime.utcnow().replace(tzinfo=utc) - timedelta(minutes=total_time)
        )
        times["future"].append(
            datetime.utcnow().replace(tzinfo=utc) + timedelta(minutes=total_time)
        )
        total_time += interval
    past = iss.at(ts.from_datetimes(times["past"]))
    future = iss.at(ts.from_datetimes(times["future"]))
    past_points = wgs84.subpoint(past)
    future_points = wgs84.subpoint(future)
    past_res = list(zip(past_points.longitude.degrees, past_points.latitude.degrees))
    future_res = list(
        zip(future_points.longitude.degrees, future_points.latitude.degrees)
    )

And yields awesome results like:

I would be glad to offer some feedback as well, so feel free to ping me anytime. I'm curious to follow the plans, so will tune in on those PR's. I understand the need for patience and some time, will gladly provide that :-)

[brandon-rhodes]

Wow, neat! Thanks for sharing that image!

So in your case there are n latitudes and n longitudes corresponding to n times — rather than, say, n locations that all want to do an observation at one single time, or at m times. That does keep things simpler, and I'm glad the current code works just fine, at least with storing those quantities as longitude and latitude array!

Given that your use case is working fine, and is a case where all the arrays have the same dimension, I guess the only final step here is the str(). I'll see what I can do!

[brandon-rhodes]

There! The str and repr now show something like:

WGS84 latitude [+0.0000 +1.0000 ... +4.0000 +5.0000] N longitude [10.0000 11.0000 ... 14.0000 15.0000] E elevation [20.0 21.0 ... 24.0 25.0] m

instead of raising ValueError. If you don't want to wait for the next release of Skyfield, you can try it out now with:

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

[wgwz]

Indeed, works like a charm! I was a bit puzzled at first though, I had to use this to pick up the new changes:

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

Looks nice in the pdb session now :-) Thanks for the fix! If you're curious, I made the image with cartopy. It's not exactly an easy package to install, but once you've got it, it works pretty well.

[brandon-rhodes]

I'm glad it's no longer raising the exception for you! I'll have to take a look at cartopy, thanks for linking toit.