Documentation Comments

[andrew-saydjari]

• [x] The docs here (https://docs.loopy.codes/SPICEKernels.jl/stable/reference/) seem much more complete than those https://ephemeris.loopy.codes/docs/examples/spice/. After having done all my reviewing, I now see that there is a top nav bar that links users to these other docs. However, I think at the end of the quickstart, there should be a link to those other places for more detailed documentation. This comment holds for all of the sub-packages within this larger repo.
• [x] I struggled trying to understand which kernels in SPICEKernels.jl were available to try to decide between. I think the answer is to just look at https://docs.loopy.codes/SPICEKernels.jl/stable/reference/#SPICEKernels.SPICEKernels, so see my above comment, because I feel like pointing people to that list should go in the upper-level (overview) docs.
• [x] No installation instructions for either HorizonsAPI.jl or HorizonsEphemeris.jl. Installation instructions for SPICEBodies.jl should also include Pkg.add. Having the main Pkg.add line on the overall readme for the EphemerisSources.jl might also be nice.
• [x] The default output of a KernelBody type called on an epoch is defined to be a "full state vector (KM, KM/S)." I recommend further spelling this out to be more clearly explained as (x,y,z,xdot,ydot,zdot) and the coordinate system that is in. It would also be helpful if the allowable keywords that can be passed were exposed in the documentation.
• [x] A docs badge linking to the "documentation website" would help with ease of navigation. I kept clicking on the documentation passing badge and feeling silly, even though in all of the subpackages, that badge does link to the relevant docs.
• [x] You have the package name Horizons.jl on the https://github.com/cadojo/EphemerisSources.jl?tab=readme-ov-file page, but it appears to be HORIZONS.jl on its github (and in your paper). Probably should change it for consistency.
• [x] I was very pleased that SPICEBodies worked both with Dates.jl and AstroTime.jl, which seems worth saying at the front in the overview docs page.
• [x] https://docs.loopy.software/HorizonsAPI.jl could not be reached from https://github.com/cadojo/EphemerisSources.jl/tree/main/lib/HorizonsAPI
• [x] On https://ephemeris.loopy.codes/lib/HorizonsAPI/dev/ I clicked the HORIZONS link and got a 404. https://ssd-api.jpl.nasa.gov/horizons.html

[tamasgal]

This seems to be stuck a little bit...

[cadojo]

I'm actively adding these changes! I have been granted admin access to this repo in the new org location, and I need to update the documentation URL. It's going to be another couple of weekends until I can get the paper items checked off; hope that's okay.

[andrew-saydjari]

That sounds just fine to me. I saw the progress you made on #12 and am glad you got integrated with JuliaAstro. We completely understand you only being able to work on it periodically.

[tamasgal]

Ah I see, perfectly fine then, sorry for the noise :) Take your time!

[andrew-saydjari]

Some initial comments

• [x] Broken link: https://github.com/JuliaAstro/HorizonsAPI.jl
• [x] Broken link: https://github.com/JuliaAstro/HorizonsEphemeris.jl
• [x] Broken link: https://github.com/JuliaAstro/SPICEBodies.jl
• [x] Broken link: https://github.com/JuliaAstro/SPICEKernels.jl
• [x] Plots in some examples (for instance https://juliaastro.org/EphemerisSources.jl/docs/dev/examples/spice/) are not rendering
• [x] Broken link: https://juliaastro.org/EphemerisSources.jl/lib/HorizonsAPI/stable
• [x] Broken link: https://juliaastro.org/EphemerisSources.jl/lib/SPICEKernels/stable
• [x] Case study would benefit from measured minus ideal, instead of the two separate plots, but that is just a preference.

[cadojo]

@andrew-saydjari ready for re-review. I should have all broken links fixed.

[andrew-saydjari]

Everything seems fixed except

• [x] You have the package name Horizons.jl on the https://github.com/cadojo/EphemerisSources.jl?tab=readme-ov-file page, but it appears to be HORIZONS.jl on its github (and in your paper). Probably should change it for consistency.

[cadojo]

Done!

[andrew-saydjari]

Agreed. My review on this issue is complete.