Handle TLE archives with multiple entries

[espg]

This PR adds TLE archive parsing to grab the correct TLE based on the specified UTC time. This is in response to #70 , where calling get_lonlatalt can give very spurious results when the orbital parameters are propagated over long time intervals (i.e., months, years, or decades). The solution is to use TLE archives... but the current version of pyorbital only grabs the first TLE and doesn't check for closest in time to the query. I've added a single new parameter, self.utctime, that is updated whenever a function requests a utctime input from the user (e.g., get_lonlatalt()). I also changed how the orbital parameters are stored in the object-- getting self.sgdp4 forces a call on self.orbit_elements, which returns the evaluation of OrbitElements(self.tle), and similarly forces evaluation of self.tle, which on read parses for the correct TLE based on the parameter utctime. In other words, the orbital parameters are dynamically calculated and returned based off of the last update to utctime , instead of created as static values that are stored in the object.

A couple of caveats:

1. There is new error behavior for poor matches between TLE parameters and requests for positional information. One of the things that was frustrating about get_lonlatalt doesn't give correct results for Terra / Aqua #70 was that there was no indication that the returned orbital positions were wrong, in some cases by a full hemisphere. As written, calling a position with a mismatch on the TLE and the utctime parameter by more than a week raises an exception. This could be changed to a warning--or a warning for small values (3 days?) and an exception for large ones (a month?)-- but there should probably be some hard check or user warning when you can tell that the returned answers will be demonstrably false. @mraspaud I'm guessing that you have some opinions on how strict these limits should be, at what point warnings vs exceptions are appropriate, and how much of a social contract for the library is in place that output is expected even when propagated over long time scales...
2. The above exception is raised for cases where no archive is provided too. Default behavior for tlefile.Read() when tle_file=None is to fetch from celestrak. That's fine and still works, but if no TLE archive is specified, and a location for a year ago is requested, an exception occurs just as it would if a TLE archive was supplied but the utctime parameter supplied by the user wasn't in archive or close to it. (Same for if the TLE Line1 and Line2 values are provided at time of object creation as strings). If the current TLE from celestrek or user string is close (within a week) to the utctime query, everything works as expected.
3. Lists and array input of datetime objects or numpy timestamps still works fine...but there is a restriction on the total spread of the timestamps. Let's say that you want to query 800 times for the location of a satellite over a day or two; that's fine, no issue--the mean of the times supplied is used to lookup the closest TLE in the archive and the calculations for the positions is run as an array. On the other hand, let's say that you pass an array of 100 datetime objects that are spread over a decade; this is a problem because there's no way to select a single TLE that is appropriate, and no obvious heuristic for how to chunk the calculations by segmenting orbital calculation updates. The way this is handled is that an error is passed if the list or array of times is spread over more than 3 days; for that, the call to the Orbital object needs to be wrapped in a loop so that the object can update the dates and TLE selection.
4. The creation signature for Orbital is unchanged. The new attribute utctime could be added as another positional argument if desired...right now it is either set as None and inferred at runtime when a time is provided, inferred from the TLE that is used at object creation, or set to datetime.now() if the most recent TLE from celestrek is used.
5. Tests have been run for this interactively... just not written and added. Guessing that determining when an exception vs warning should be raised will inform how the tests are run.

• Closes get_lonlatalt doesn't give correct results for Terra / Aqua #70
• Tests added
• Tests passed
• Passes flake8 pyorbital
• Fully documented (comments in code, no user documentation)

[ghost]

DeepCode failed to analyze this pull request

Something went wrong despite trying multiple times, sorry about that.
Please comment this pull request with "Retry DeepCode" to manually retry, or contact us so that a human can look into the issue.

[mraspaud]

@epsg Thanks a lot for putting this all together! I haven't looked at the code yet, but it sounds great!

1. regarding limits, I would issue a warning after 3 days and raise an exception after 1 week, per default. Then we could maybe add a parameter to allow the user to propagate further (still issuing a warning?)
2. sounds good
3. regarding mean, I would like if possible to use the median instead so that erraneous times can't weigh in so much (although maybe that's overkill)
4. I have to think a bit more about the implications of the new utctime attribute
5. Good that you have tests (almost) ready :)

I will try to have a look at the code shortly!

[espg]

@mraspaud any chance you can take a look at this when you have a moment? Last year I moved and switched jobs twice after I started this PR, but things have settled down a bit now and I would like to get it merged.

To pick up the conversation and items from last round:

1. Archives warn at 3 days, and error at 7.
2. Same behavior here, although I changed things so that the utc time is stripped out of TLE's that are specified on the function itself. This seemed clean, since it meant that tests that specify TLE's explicitly will still pass without modification (as opposed to adding an optional utc argument and changing all the tests).
3. This still uses mean. Mean is a built in function for numpy arrays, and I don't think that there's that much bias over the window that is already enforced. If you have strong feelings on this, I can change it though.
4. Don't think this breaks anything as is, but not sure-- there's a few test that are failing on my branch related to AttributeError: 'OrbitElements' object has no attribute 'an_time', but I'm not sure this is related. There's a setter/getter for OrbitElements, so it should be able to access everything as per normal, but perhaps there's another property that needs explicit pass through that I'm missing.
5. Unit tests are done (and pass)

let me know what you think...

[mraspaud]

Ok, a lot of good things here, thanks a lot for taking this up again!

So I had a good look at the code, and the principles are sound.
I would like though to refactor things a little bit in the spirit if SRP (single responsability principle):

1. The Orbital class is designed to work with one set of tle only, and we could probably keep it like this so as not to complicate things too much. For this, I propose having a new class called eg OrbitalCollection, or OrbitalArchive that would take the tle files to parse, and that would implement a version of get_lonlatalts that would select the right Orbital instance from an internal list/dictionary and then call that instance's get_lonlatalts.
2. The parsing of the epoch that you implement is more or less already implemented in the Tle class, why not use that directly? So if you have, say, your list of tles in an archive.tle file, we could implement a class method for Tle that would return a list of parsed tles instead of just the first one. That way it should be possible to just use the epoch attribute of the Tle instance. For example the class method could look like this:

@classmethod
def create_tle_list(cls, platform, tle_file):
    uris, open_func = _get_uris_and_open_func(tle_file=tle_file)
    tles = []
    for tle in _get_tles_from_uris(uris, open_func, platform=platform, only_first=False):
        line1, line2 = tle.split("\n")
        tles.append(cls(platform, line1=line1, line2=line2))
    return tles

3. The tests look good for testing the boundaries, but I miss the tests for the new functionality, namely that for example the right tle lines are chosen for a given utc time.

But again, great ideas! let's keep the ball rolling this time :)

[mraspaud]

Here, could we check instead that both min and max times are within 3 days of the epoch instead? or do you think it would be too restrictive?