qpv-research-group / rayflare

Open-source, integrated optical modelling of complex stacks
Other
33 stars 12 forks source link

[JOSS] Documentation (RTD) issues #31

Closed kandersolar closed 3 years ago

kandersolar commented 3 years ago

Here is a list of the issues I noticed while browsing the documentation. All of them are pretty minor IMHO.

Index

Examples

Function/Class docs

xref https://github.com/openjournals/joss-reviews/issues/3460

phoebe-p commented 3 years ago

Re: the first issue, for me it looks like this? Could this just have been a loading issue? I'm looking at https://rayflare.readthedocs.io/en/latest/, were you at a different URL? Screenshot from 2021-08-10 14-54-19

kandersolar commented 3 years ago

Strange -- clicking that link still showed me the _images/poster.png text instead of the image, but when I hard-refreshed the page the image appeared. I'm willing to chalk it up to browser strangeness, so I'll cross it off the original list.

phoebe-p commented 3 years ago

To-do list for my own use:

phoebe-p commented 3 years ago

@kanderso-nrel I think these issues, apart from the final one (various functions don't have docstrings, i.e. the hardest one), were resolved by PR #33 + some subsequent changes to devel to update the documentation further. Currently, I think all the functions a user would actually want to call directly (in addition to many of the more 'internal' functions) have docstrings. Obviously, in the long term it would be good for all functions to have docstrings since it will help people who want to understand the code/contribute.

kandersolar commented 3 years ago

Okay, makes sense! If there's not a good use case for a user to call those functions themselves, one option would be to make them private by putting an underscore at the front of the function name. I think that will keep them from showing up in the sphinx docs, as well as in IDE autocomplete suggestions and such. NBD though, ok with me to consider this issue addressed and close it

phoebe-p commented 3 years ago

Ah that's a good idea. At least for now this probably makes sense to avoid loads of undocumented functions showing up in the Sphinx docs.