Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[JOSS] Documentation (RTD) issues #31

Closed
kandersolar opened this issue Aug 5, 2021 · 6 comments
Closed

[JOSS] Documentation (RTD) issues #31

kandersolar opened this issue Aug 5, 2021 · 6 comments
Assignees

Comments

@kandersolar
Copy link

kandersolar commented Aug 5, 2021

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

Index

  • The bottom of index.rst is supposed to show the poster, but it doesn't render correctly on RTD: probably just some cache/loading issue on my first load; a hard refresh fixed it
    image

Examples

  • Examples/rcwa_tmm_validation.html: figure titles are labeled "deg" but I think are actually in radians
  • Examples/compare_models_3Jsolarcell.html: The figure labels are overlapping, consider using (sharex=True, sharey=True) or tight_layout()

Function/Class docs

  • matrix_formalism.process_structure.process_structure: duplicate "options" in docstring
  • matrix_formalism.multiply_matrices.make_v0: phi_sym not documented
  • matrix_formalism.ideal_cases.lambertian_matrix: structpath not documented, extra "options"
  • matrix_formalism.ideal_cases.mirror_matrix: structpath not documented
  • transfer_matrix_method.lookup_table: structpath not documented
  • rigorous_coupled_wave_analysis.rcwa.get_reciprocal_lattice: sphinx rendering issue
  • rigorous_coupled_wave_analysis.rcwa.rcwa_structure: in constructor and methods, "options" not documented
  • rigorous_coupled_wave_analysis.rcwa.rcwa_structure.get_fields_z_integral: pol is not an actual parameter
  • analytic module is not documented
  • textures module is not documented
  • various functions don't have docstrings

xref openjournals/joss-reviews#3460

@phoebe-p phoebe-p self-assigned this Aug 9, 2021
@phoebe-p
Copy link
Member

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
Copy link
Author

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
Copy link
Member

phoebe-p commented Aug 10, 2021

To-do list for my own use:

  • Examples/rcwa_tmm_validation.html: figure titles are labeled "deg" but I think are actually in radians

  • Examples/compare_models_3Jsolarcell.html: The figure labels are overlapping, consider using (sharex=True, sharey=True) or tight_layout()

  • matrix_formalism.process_structure.process_structure: duplicate "options" in docstring

  • matrix_formalism.multiply_matrices.make_v0: phi_sym not documented

  • matrix_formalism.ideal_cases.lambertian_matrix: structpath not documented, extra "options"

  • matrix_formalism.ideal_cases.mirror_matrix: structpath not documented

  • transfer_matrix_method.lookup_table: structpath not documented

  • rigorous_coupled_wave_analysis.rcwa.get_reciprocal_lattice: sphinx rendering issue

  • rigorous_coupled_wave_analysis.rcwa.rcwa_structure: in constructor and methods, "options" not documented

  • rigorous_coupled_wave_analysis.rcwa.rcwa_structure.get_fields_z_integral: pol is not an actual parameter

  • analytic module is not documented

  • textures module is not documented

  • various functions don't have docstrings

@phoebe-p
Copy link
Member

@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
Copy link
Author

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
Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants