Different element sets - poliastro/poliastro GitHub Wiki

Right now I'm considering just one element set: (a, e, i, Ω, ω, ν). It works fine with elliptical orbits but is definitely useless for parabolic orbits - I would use the semilatus rectum p for that. Plus, I might want to use the mean anomaly M instead but I can't.

Idea: Maybe a simple fix would be: return to accepting tuples (i.e. revert 64dc66c) and assume p if ecc == 1.0 (or even within an interval). Plus, include an extra parameter anomaly=='true' or 'mean' and assume the corresponding value. I can also use an extra parameter for the shape='sma' or 'p'. All the rest remains equal.

The good thing is that it makes programming easy: right now State.__init__ only accepts (r, v) and the classical orbital elements, if they were precomputed, are set from outside:

def from_elements(...):
    # compute r and v
    ss = cls(attractor, r * u.km, v * u.km / u.s, epoch)
    ss._elements = a, ecc, inc, raan, argp, nu

This should be avoided though, as it does not look nice.

Making them @properties is still the way to go, because I get a nice write protection:

>>> molniya.a = 1
AttributeError: can't set attribute

The point is that, if we allow for different element sets, it may be not as easy as

@property
def a(self):
    return self.elements[0]

Options:

  • Use a default element set as the internal representation (I don't say canonical to avoid confusion with Delaunay and Poincaré elsets) and convert the rest from those. The bad thing is that I have two default representations: (r, v) and nonsingular elset, the good thing is more straightforward code.
  • All elements are virtual - the selected ones are set on instantiation.

Probably most of the properties will look something like

@property
def a(self):
    try:
        return self._a
    except:
        self._a = ...
        return self._a

Branching the code for every single property doesn't feel very good to me, but I'll have to deal with it if there is no other option. This is not much better:

@property
def a(self):
    # self._a defaults to none
    return self._a or self._compute_a()

For the constructor, I contemplate two options too:

  • Use an extra parameter, for example kind='classical' or kind='equinoctial'. Some set of fixed options, but I still think I limit myself too much, but still the code is simpler.
  • Use keyword args and detect names. This looks awesome, and difficult to implement.

New ideas

To solve the parabolic orbit case, if I already have a State.circular classmethod I could probably create a State.parabolic method. Circular are special cases of elliptical, and parabolic orbits are special per se.

On the other hand, perhaps instead of using State.from_elements I should write State.from_keplerian so I can allow other element sets in the future.

Finally, the scipy.signal LTI objects have similar concerns, and they noticed that a single class was holding too many properties (which already happens to State objects). This is the relevant PR:

https://github.com/scipy/scipy/pull/4576

This departs from what I took on that same code (see scipy/scipy#2756) but it is an interesting approach. In particular, I like the way the class is selected (broken link)

Therefore, parent class defines all properties in terms of conversions and child subclasses override "their" properties.

Note: The scipy.signal package has changed a lot lately, and in particular see this PR for an idea of what I am trying to attempt:

https://github.com/scipy/scipy/pull/6224

In [Brainstorming] it is stated that subclassing would be problematic because a Maneuver can return an hyperbolic orbit from an elliptic one, but this is not intended to subclass by type of orbit - only by kind of instantiation or definition.

However, if it does not obfuscate the code I would make these subclasses private so the user only uses State. This can wait after 0.3 probably.