-
Notifications
You must be signed in to change notification settings - Fork 11
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
common/lib/cv: Test docs #189
common/lib/cv: Test docs #189
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Conditionally approved, assuming that all my comments are addressed.
opencsp/common/lib/cv/spot_analysis/SpotAnalysisOperablesStream.py
Outdated
Show resolved
Hide resolved
opencsp/common/lib/cv/spot_analysis/SpotAnalysisOperablesStream.py
Outdated
Show resolved
Hide resolved
opencsp/common/lib/cv/spot_analysis/SpotAnalysisOperablesStream.py
Outdated
Show resolved
Hide resolved
""" | ||
# "ChatGPT 4o-mini" assisted with generating this docstring. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"assisted"
@@ -5,15 +5,53 @@ | |||
import opencsp as opencsp | |||
import example as example | |||
|
|||
# TODO: why aren't these imported from import opencsp as opencsp above |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We generally don't want to import the world when importing opencsp, so we don't auto-import everything in the base /init.py file. Instead, we depend on the developer to import everything they need.
We should replace the 'from opencsp... import ...' in lines 9-14 with 'import opencsp...', to match the rest of this file's imports.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, please file an issue so we can take care of this in a follow-on PR.
942e4c7
to
b3a4b08
Compare
8c44dad
to
d253607
Compare
Purpose
Test and complete docs in common/lib/cv
Summary of changes
Update test_DocStringsExists to include all members of common/lib/cv. Add missing doc strings
Implementation notes
Ran
pytest -s -k test_docstrings
Submission checklist
develop
, notmain
opencsp/test/test_DocStringsExist.py
are verified to include this change or have been updated accordinglydoc/
are verified to include this change or have been updated accordinglyAdditional information
Please provide any additional information here.