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

177 RST stubs for ReadTheDocs #178

Conversation

bbean23
Copy link
Collaborator

@bbean23 bbean23 commented Oct 29, 2024

Purpose

Please briefly describe the purpose of this pull request. If this fixes a github issue please include e.g. fixes #22 so that the issue gets closed automatically when this is merged.

Summary of changes

  • provided RST stubs for SpotAnalysis classes
  • updated docstrings
  • added classes to test_DocStringsExist.py

Implementation notes

Added AutoSummary to Sphinx for generating class name lists at the top of the documentation files.

Submission checklist

  • Target branch is develop, not main
  • Existing tests are updated or new tests were added
  • opencsp/test/test_DocStringsExist.py are verified to include this change or have been updated accordingly
  • .rst file(s) under doc/ are verified to include this change or have been updated accordingly

Additional information

none

@bbean23 bbean23 added the documentation Improvements or additions to documentation label Oct 29, 2024
@bbean23 bbean23 self-assigned this Oct 29, 2024
@bbean23 bbean23 changed the title RST stubs for ReadTheDocs 177 RST stubs for ReadTheDocs Oct 29, 2024
@bbean23 bbean23 linked an issue Oct 29, 2024 that may be closed by this pull request
1 task
Copy link
Collaborator

@e10harvey e10harvey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for these changes @bbean23. I left some minor comments below. Please reorganize these rst files to match the directory structore of the code base, like this:
doc/source/library_reference/library/SpotAnalysis -> doc/source/library_reference/common/lib/cv/spot_analysis. I'd be happy to open a PR into your fork with these changes.

Copy link
Collaborator Author

@bbean23 bbean23 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe I've addressed all your comments. Please note that by "addressed" I sometimes mean "argued against", so please take a look at my comments to see if you agree or not.

opencsp/test/test_DocStringsExist.py Outdated Show resolved Hide resolved
opencsp/common/lib/cv/SpotAnalysis.py Outdated Show resolved Hide resolved
opencsp/common/lib/cv/CacheableImage.py Outdated Show resolved Hide resolved
@bbean23 bbean23 requested a review from e10harvey November 1, 2024 20:05
Copy link
Collaborator

@e10harvey e10harvey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @bbean23! I think this can be merged after the headings are made consistent with the rest of the library reference and "also maaaaaybe look at" is reworded to "also see".

@e10harvey e10harvey mentioned this pull request Nov 13, 2024
25 tasks
e10harvey
e10harvey previously approved these changes Nov 13, 2024
@bbean23 bbean23 force-pushed the 177-code-feature-spotanalysis-readthedocs-stubs branch from 5485cd6 to 0a28c63 Compare November 13, 2024 19:28
@bbean23 bbean23 force-pushed the 177-code-feature-spotanalysis-readthedocs-stubs branch from c1bbf28 to 9bcf584 Compare November 15, 2024 19:39
@bbean23
Copy link
Collaborator Author

bbean23 commented Nov 15, 2024

@e10harvey Please review again. I made some small changes in order to get the build to pass and documentation to render correctly:

  • fixed docstrings in CacheableImage to be compatible with ReStructuredText
  • changed "AbstractSpotAnalysisImagesProcessor" to "AbstractSpotAnalysisImageProcessor"

@bbean23 bbean23 requested a review from e10harvey November 15, 2024 20:16
@e10harvey e10harvey merged commit 4e6ddfa into sandialabs:develop Nov 18, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Code Feature]: SpotAnalysis ReadTheDocs stubs
2 participants