My main concern in this review is the state of the documentation. This is a great start, and the code itself is extensively commented, but the the external documentation could use some tweaks. I have a few nice-to-have and a few must-haves.
The must-haves are:
Documentation for the ForwardModel class, both in the code and in the external documentation. This seems to be the major class in the package, and although your examples rely on it heavily, it and its methods are not currently included in the external documentation. Several docstrings in the code are also incomplete, including the one for nemesispy.ForwardModel.calc_disc_spectrum, which is a key method in the examples.
Inclusion of other docstrings in the external API documentation. The docstrings in the code are very good and seem to be formatted correctly for Sphinx to read them, but many of the functions/methods are not currently rendered in the external documentation. Right now you use .. autofunction:: to create entries for a handful individual functions, but considering that the docstrings for many more functions already exist, it would be nice to either include all of them. This could be done by including each individually with their own autofunction line, or by using something like automodule to include several at once.
Some nice-to-haves:
Right now the documentation only exists as a PDF file. However, I see that you are using Sphinx for docs generation, so you're well on the way to being able to host it as a website instead. It'd be really nice if you could deploy an html version of the documentation to a github page or readthedocs, that way users can follow links between sections navigate more easily. I also see that you have a github action setup to deploy something to github pages, but right now this link returns a 404 error.
The paper mentions retrievals, and there's an undocumented function nemesispy.retrieval.read_stats that seems to have something to do with parsing the results of a fit. It'd be great if this could be documented, and maybe if an example retrieval could be included as a complete end-to-end demonstration of how someone might use nemesispy to analyze spectral data.
Let me know if these suggestions make sense- I really like the package structure and the comments in the code itself, so hopefully it won't be too challenging to enhance the docs!
My main concern in this review is the state of the documentation. This is a great start, and the code itself is extensively commented, but the the external documentation could use some tweaks. I have a few nice-to-have and a few must-haves.
The must-haves are:
nemesispy.ForwardModel.calc_disc_spectrum
, which is a key method in the examples... autofunction::
to create entries for a handful individual functions, but considering that the docstrings for many more functions already exist, it would be nice to either include all of them. This could be done by including each individually with their ownautofunction
line, or by using something likeautomodule
to include several at once.Some nice-to-haves:
nemesispy.retrieval.read_stats
that seems to have something to do with parsing the results of a fit. It'd be great if this could be documented, and maybe if an example retrieval could be included as a complete end-to-end demonstration of how someone might usenemesispy
to analyze spectral data.Let me know if these suggestions make sense- I really like the package structure and the comments in the code itself, so hopefully it won't be too challenging to enhance the docs!