Add Orbit.from_Horizons convenience function

[moeyensj]

This PR adds a classmethod to the Orbit class that loads an orbit from JPL Horizons. This is functionality that is extremely useful and is based on THOR's method of the same name but in the absence of an adam.core package, I duplicate it here and modify it slightly to fit the precovery context.

You can now load an orbit as follows:

from astropy.time import Time
from precovery.orbit import Orbit

t0 = Time(59000, scale="utc", format="mjd")
orbit = Orbit.from_Horizons("Ivezic", t0)

The downside is that we add astroquery and astropy as dependencies, though given how commonplace these packages are I think that should be okay.

[akoumjian]

Are there already similar methods like this on the orbit class where we are fetching remote data? If not, I'm wondering if we can potentially break this up into two pieces, or a standalone query function that returns the Orbit. I realize that's stylistic only.

My thinking is that we have a bunch of .from_x methods that are stateless, initialization factories that once written correctly will basically always work inside someone else's code. This method, however, can fail for a number of networking or service related reasons that have nothing to do with how the method is used. The person using it might not even have a sense that a network boundary is occurring.

My second thought on it is that the primary action here is a query. It's kind of incidental that the return value is an Orbit class, since that's really just some validation typing that we use to make sure data is being declared properly. So in my mind, it's an opinionated utility query function (of which we might adopt others?) and likely belong inside something like orbit.query or orbit.fetch module as a fetch_horizon() or some such.

None of that is blocking, just my two cents. Also, let's prioritizing factoring out adam.core as soon as we can.

[moeyensj]

I really like the idea of separating query utilities out a little more. It hadn't occurred to me that it might confuse the user into thinking that a function like from_horizons should always work.

While I'm working on the overhaul for THOR I can implement the suggested structure. Does something like this look okay? Presumably, we will use the core of THOR for adam.core at some point anyway, so while I'm working on that and making sure what exists is actually useful I can also implement this structure (most of it already exists just needs to be moved around a little).

thor
  orbits
    __init__.py
    orbits.py
    query 
      __init__.py
      horizons.py
      sbdb.py
      mpc.py
      adam.py (I figure at some point we will store orbits of our own)

From the user standpoint it would look like:

from astropy.time import Time
from thor.orbits.query import query_horizons
from thor.orbits.query import query_sbdb

t0 = Time([59000.0], scale="utc", format="mjd")

orbits = query_sbdb(["Eros", "Ivezic"], t0)

[akoumjian]

I like that.

On Thu, Nov 17, 2022 at 10:11 AM Joachim Moeyens @.***> wrote:

I really like the idea of separating query utilities out a little more. It hadn't occurred to me that it might confuse the user into thinking that a function like from_horizons should always work.

While I'm working on the overhaul for THOR I can implement the suggested structure. Does something like this look okay? Presumably, we will use the core of THOR for adam.core at some point anyway, so while I'm working on that and making sure what exists is actually useful I can also implement this structure (most of it already exists just needs to be moved around a little).

thor orbits init.py orbits.py query init.py horizons.py sbdb.py mpc.py adam.py (I figure at some point we will store orbits of our own)

From the user standpoint it would look like:

from astropy.time import Time from thor.orbits.query import query_horizons from thor.orbits.query improt query_sbdb

t0 = Time([59000.0], scale="utc", format="mjd")

orbits = query_sbdb(["Eros", "Ivezic"], t0)

— Reply to this email directly, view it on GitHub https://github.com/B612-Asteroid-Institute/precovery/pull/47#issuecomment-1318779887, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAFTGJDAE4NVGGPQV6ZQS5TWIZDL7ANCNFSM6AAAAAASC36BMA . You are receiving this because your review was requested.Message ID: @.***>

[moeyensj]

I've turned this into an issue on the THOR repository (https://github.com/moeyensj/thor/issues/96).