Cannot use skyfield.api.load.tle .items() function

[cweickhmann]

I came across a slightly odd behaviour when trying to filter the dict of objects loaded using skyfield.api.load.tle.

Let's say, I want to load the list of active satellites from Celestrak and among them filter for a bunch of Cubesats (let's say AEROCUBE), I run in trouble:

import skyfield.api as skyapi
loader = skyapi.Loader(r"~/.skyfield-data")
active_satellites_url = "http://celestrak.com/NORAD/elements/active.txt"
satellites = loader.tle(active_satellites_url)

k, v = satellites.items()

results in

---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
<ipython-input-10-ec9f8531e0e7> in <module>
----> 1 k, v = satellites.items()

ValueError: too many values to unpack (expected 2)

But it's listed as a dict and the .keys() and .values() functions work as expected. And .items() does in fact return a dict_items object as expected:

>>> type(satellites)
dict
>>> satellites.keys()
dict_keys([900, 'CALSPHERE 1', 902, 'CALSPHERE 2', 1361, 'LCS 1', 1512, 'TEMPSAT 1', 1520, 'CALSPHERE 4A', 2826, 'OPS 5712 (P/L 160)', 'OPS 5712', 'P/L 160', 2872, 'SURCAL 159', ...
>>> satellites.values()
dict_values([<EarthSatellite 'CALSPHERE 1' number=900 epoch=2020-02-25T19:20:52Z>, <EarthSatellite 'CALSPHERE 1' number=900 epoch=2020-02-25T19:20:52Z>, <EarthSatellite 'CALSPHERE 2' number=902 epoch=2020-02-25T11:56:49Z>, ...
>>> satellites.items()
dict_items([(900, <EarthSatellite 'CALSPHERE 1' number=900 epoch=2020-02-25T19:20:52Z>), ('CALSPHERE 1', <EarthSatellite 'CALSPHERE 1' number=900 epoch=2020-02-25T19:20:52Z>), (902, <EarthSatellite 'CALSPHERE 2' number=902 epoch=2020-02-25T11:56:49Z>), ('CALSPHERE 2', <EarthSatellite 'CALSPHERE 2' number=902 epoch=2020-02-25T11:56:49Z>), ...

If I create a test dict, it works well:

test = {"a": (1,2,3), "b": (2,3,4)}
k, v = test.items()

I am using

• Python 3.8.1 (tags/v3.8.1:1b293b6, Dec 18 2019, 23:11:46) [MSC v.1916 64 bit (AMD64)]
• Skyfield 1.17

[brandon-rhodes]

I am not sure that Python supports k, v = test.items() as an operation — at least I don't think I've ever seen it. What would it return, since a dictionary has several keys and several values?

[cweickhmann]

Yes, it does and I use it a lot, for instance for filtering. See the first example here.
I just wonder why it doesn't work here...

[brandon-rhodes]

The first example says:

for k, v in knights.items():

which is very different than k, v = test.items()?

[cweickhmann]

True, I'll edit this in OP...

In [1]: test = {"a": (1,2,3), "b": (4,5,6)}                                         

In [2]: k, v = test.items()                                                         

In [3]: k                                                                           
Out[3]: ('a', (1, 2, 3))

In [4]: v                                                                           
Out[4]: ('b', (4, 5, 6))

It worked by accident because the two-item iterator was unpacked into two values. Not what I wanted to do.
But consider this:

In [5]: for k, v in test.items(): 
   ...:     print(k, v) 
   ...:                                                                             
a (1, 2, 3)
b (4, 5, 6)

That's what I meant to show. And here's what is odd:

In [8]: satellites[900]                                                             
Out[8]: <EarthSatellite 'CALSPHERE 1' number=900 epoch=2020-02-27T20:23:16Z>

In [9]: satellites["CALSPHERE 1"]                                                   
Out[9]: <EarthSatellite 'CALSPHERE 1' number=900 epoch=2020-02-27T20:23:16Z>

both work, but the key seems to be the int 900:

In [5]: import skyfield.api as skyapi 
   ...: loader = skyapi.Loader(r"~/.skyfield-data") 
   ...: active_satellites_url = "http://celestrak.com/NORAD/elements/active.txt" 
   ...: satellites = loader.tle(active_satellites_url) 
   ...:  
   ...: for k, v in satellites.items(): 
   ...:     print(type(v)) 
   ...:     if "O3B" in k: 
   ...:         print(k, v) 
   ...:                                                                             
<class 'skyfield.sgp4lib.EarthSatellite'>
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-5-3eef8217fce2> in <module>
      6 for k, v in satellites.items():
      7     print(type(v))
----> 8     if "O3B" in k:
      9         print(k, v)
     10 

TypeError: argument of type 'int' is not iterable

So filtering is problematic because the IDs and string labels come interleaved.
I now understand why I ran into this problem, but it is still odd and unexpected from a user perspective.

In [10]: for i, (k, v) in enumerate(satellites.items()): 
    ...:     print(k) 
    ...:     if i > 10: break 
    ...:                                                                            
900
CALSPHERE 1
902
CALSPHERE 2
1361
LCS 1
1512
TEMPSAT 1
1520
CALSPHERE 4A
2826
OPS 5712 (P/L 160)

What kind of structure is below this? Is it simply a dict with two entries per satellite?

[brandon-rhodes]

It is a dictionary that allows lookup by satellite number or satellite name:

https://rhodesmill.org/skyfield/api-iokit.html#skyfield.iokit.Loader.tle

Would it help if I expanded the discussion in the main "Earth Satellites" document:

https://rhodesmill.org/skyfield/earth-satellites.html

— instead of relying on folks to click through to the tle() documentation for the details? Also, maybe I should add an alternative routine that does not even create a dictionary but instead just returns a list. I now regret building a dictionary for users; I should instead have just returned a list and showed example code for creating a lookup dictionary in case users wanted one.

[cweickhmann]

No, don't regret it just because I didn't get it right away ^^.
I think dict is fine because it is powerful. What made it confusing was the fact that I was expecting to get only string keys but got the mix of int and string.
What about returning a class that provides the dict and maybe a slim filter function?
Something along the lines of (in iokit.py):

from collections import UserDict
class SatelliteList(UserDict):
    """Using `collections.UserDict`, this wrapper behaves mostly like
    a dict but provides a couple of extra functions.
    """
    def filter(self, pattern):
        filtered_dict = {k: v for k, v in self.data.items() if isinstance(k, str) and pattern in k}
        return filtered_dict

It simply makes sure the pattern is only checked against str keys and the user does not need to care.

I think it's not too difficult to include some added functionality. There are other prototypes in collections.abc that might make the use of two keys per satellite more encapsulated towards the user (someone played with those).

I've forked your repo and can provide you with a pull request if you're interested.

[brandon-rhodes]

That's an interesting idea! My first thoughts are:

1. I would rather resolve the confusion by making Skyfield simpler, rather than more complicated (in this case, adding both a class and a new method, instead of returning an object like a dict or list that users would find familiar).
2. If someone wants only the names or satellite numbers, or simply wants all the satellites in the TLE file, then forcing users to receive the full satnum-and-name dict and then build a second dictionary with only the elements they want is going to be slower for them than if Skyfield simply returned what they wanted to begin with.
3. I think readability is hurt somewhat. If folks reading code see a call to a Skyfield filter() function then they will have to go look up how it works. Whereas if we switch to simply returning a list of satellites, and encourage users to write things like byname = {sat.name: sat for sat in sats} as the next line of code, then we'll be encouraging them to use plain old easily readable Python that any Python programmer will be able to understand without looking anything up in the Skyfield manual.

So in a couple of weeks, when I'll have time to look over the code, I'll probably create a new replacement for tle(). I'll have to leave the old routine around forever, of course, because there is old code that still calls it. But I'll switch the docs to talking only about the new one, which will return a simple list of satellites. The act of creating a dictionary is so simple in Python that I should have just left that to users; creating a hybrid-typed dictionary was a bad move that makes the call more expensive when the user will almost never need both keys.

At least I've learned a bit about programming since I wrote that function. Alas!

[cweickhmann]

Thanks for the fruitful discussion. Thats a good plan.
If there's anything I can contribute, let me know.

[brandon-rhodes]

There, that should help! The improved, simplified routine should be available the next time I release Skyfield.

[cweickhmann]

Thank you very much! :-)