New Horizons API

[LuEdRaMo]

In order to address https://github.com/PerezHz/HORIZONS.jl/issues/31, this PR translates functions smb_spk, smb_spk_ele and vec_tbl to the new Horizons API.

[LuEdRaMo]

@PerezHz what are your thoughts on this PR? Also, I have a couple of questions:

1. Currently, I separate the original functions from the new ones via a ::Val{API} argument but, Is it worth keeping the telnet methods, since those require installing telnet externally while the new ones do not?
2. Should I go ahead and implement other JPL APIs such as these?

[PerezHz]

Hey @LuEdRaMo! Many thanks for this PR! I'll have a look to the details of this PR as soon as I can.

I would actually be in favor of scrapping the telnet methods (thus eliminating an external dependency in favor of a safer and more modern data-fetching API); ie I'm in favor of switching to the new HTTP API completely. Nostalgia-driven users won't mind using an older HORIZONS.jl version 🙃.

Regarding your second question, sure go ahead!

[PerezHz]

Just a small caveat: we should add a warning to the users explaining that we switched APIs from telnet to HTTP.

[LuEdRaMo]

I have a few more comments/questions:

1. Should we bump the major or the minor version?
2. Except for horizons(), the other functions have been adapted to the new API.
3. The only function that I have not adapted yet is vec_tbl_csv, is it worth keeping it? I feel that it could belong to a DataFrames extension.
4. I still have to update README.md.
5. I've written new tests.
6. So far, I've implemented 3 of the JPL APIs (Horizons, SBDB and Radar Astrometry), I think the rest can be the subject of future PRs.

[PerezHz]

Should we bump the major or the minor version?

Before 1, versions 0.x are considered breaking among themselves, so bumping the minor version should enough in this case.

Except for horizons(), the other functions have been adapted to the new API.

Okay, eventually the telnet method to connect to Horizons will be removed by JPL, but let's keep it for the time being.

The only function that I have not adapted yet is vec_tbl_csv, is it worth keeping it? I feel that it could belong to a DataFrames extension.

I agree that vec_tbl_csv is not essential and perhaps a bit redundant; feel free to deprecate it.

I still have to update README.md.

So far it has worked as the documentation of the package, so on this point I do think it's worth to update together with this PR.

I've written new tests.

Many thanks for adding those!

So far, I've implemented 3 of the JPL APIs (Horizons, SBDB and Radar Astrometry), I think the rest can be the subject of future PRs.

I think that's more than enough for what the package is already covering; I agree we can leave the rest of the JPL APIs for future PRs, I'd suggest to just open an issue here regarding that point.

[PerezHz]

Btw, julia 1.0 tests are failing, can you update the CI.yml to test julia 1.6, 1 and nightly?

[LuEdRaMo]

In the last commits I've addressed all your comments @PerezHz. I think this PR is ready for review.

[PerezHz]

@LuEdRaMo Just left one last comment. Let me know when this is ready, and then we should be good to go!

[PerezHz]

Deleted my last comment, the branching was swapped; will update my suggestion in a bit 😅

[PerezHz]

I went ahead and committed the suggestion; if we get the green lights I'll merge, thanks a lot!

[LuEdRaMo]

I went ahead and committed the suggestion; if we get the green lights I'll merge, thanks a lot!

Thank you @PerezHz and sorry for the delay. On my side everything looks fine...

I'm excited about the applications of this PR in NEOs.

[PerezHz]

Thank you @PerezHz and sorry for the delay. On my side everything looks fine...

No worries, and great catch on the breaking changes warning!

I'm excited about the applications of this PR in NEOs.

Indeed, me too! 😄