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.

Sign up for GitHub

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

Jump to bottom

Extensive revisions to docstrings throughout the stellarphot package #97

Merged
merged 35 commits into from
Jun 14, 2023
Merged

Extensive revisions to docstrings throughout the stellarphot package #97

merged 35 commits into from
Jun 14, 2023

Conversation

JuanCab
Copy link
Contributor

@JuanCab JuanCab commented Jun 9, 2023

I basically went through the entire package and added docstrings for all public functions and packages where they were missing as well as trying to revise incomplete docstrings.

I got a lot of help with GitHub Copilot and while I tried to perform reality checks on the meanings of things, I may well be in error in some places.

Some issues to watch for:

  • My inline math for calculate_noise in photometry.py is not rendering properly in sphinx construction of documentation.
  • I need the author of the code to review as much of the documentation as they can and make sure the descriptions are accurate.
  • I am a bit concerned that in some places it just says 'array-like' instead of 'array of floats' etc. Do we need more precise language in some places?

JuanCab and others added 12 commits June 7, 2023 12:41
@JuanCab
Tweaked gitignore to avoid git archiving files from nova editor subdi…
869af0d 
…rectory.
@JuanCab
Added missing docstrings for analysis, core, and source_detection code.
d67a29e
@JuanCab
Revised docstrings in analysis package to be more complete (include r…
def44dd 
…eturned values)
@JuanCab
Revised docstrings for the photometry and visualization packages.
bb5089e
@JuanCab
Typo correction and removal of documentation causing pytest failures.
74cc196
Tweaks to differential photometry docstrings and comments.
c3f0365
Additional tweaks to differential photometry docstrings and comments.
9222838
Added docstrings to aij_plots and comparison_function in visualizatio…
85dc3c3 
…n package.
Updated docstrings to visualization package functions and classes.
ac6b439
Docstring updates made for io Package.
4126d41
Tweaked the docstrings for calculate_noise to make a bit more compact.
574481b
Still tweaking calculate_noise docstring math, still not rendering co…
ebd5236 
…rrectly.
@mwcraig
Copy link
Contributor

mwcraig commented Jun 12, 2023

Thanks, @JuanCab -- reviewing

mwcraig
mwcraig requested changes Jun 12, 2023
View reviewed changes
Copy link
Contributor

@mwcraig mwcraig left a comment

Choose a reason for hiding this comment

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

This is a pretty epic PR! We may need to discuss some of the suggested changes. If there ones that you agree with I would go ahead and accept them. After doing that you will want to pull those commits from github to your local clone.

stellarphot/analysis/exotic.py Outdated Show resolved Hide resolved
stellarphot/analysis/exotic.py Outdated Show resolved Hide resolved
stellarphot/analysis/exotic.py Outdated Show resolved Hide resolved
stellarphot/analysis/exotic.py Show resolved Hide resolved
stellarphot/analysis/exotic.py Outdated Show resolved Hide resolved
stellarphot/visualization/seeing_profile_functions.py Outdated Show resolved Hide resolved
stellarphot/visualization/seeing_profile_functions.py Outdated Show resolved Hide resolved
stellarphot/visualization/transit_plots.py Outdated Show resolved Hide resolved
stellarphot/visualization/transit_plots.py Outdated Show resolved Hide resolved
stellarphot/visualization/transit_plots.py Show resolved Hide resolved
mwcraig
mwcraig requested changes Jun 12, 2023
View reviewed changes
Copy link
Contributor

@mwcraig mwcraig left a comment

Choose a reason for hiding this comment

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

This is a pretty epic PR! We may need to discuss some of the suggested changes. If there ones that you agree with I would go ahead and accept them. After doing that you will want to pull those commits from github to your local clone.

JuanCab and others added 15 commits June 12, 2023 14:13
@JuanCab
Working my way through Matt's comments on PR#97.
18823e6
@JuanCab
Fixed docstring error noted by matt.
ffd788b
@JuanCab @mwcraig
Update stellarphot/analysis/transit_fitting.py
22e4deb 
Accepted this suggestion by Matt on PR#97.

Co-authored-by: Matt Craig <[email protected]>
@JuanCab @mwcraig
Update stellarphot/core.py
fde3915 
Accepted this suggestion by Matt on PR#97.

Co-authored-by: Matt Craig <[email protected]>
@JuanCab @mwcraig
Update stellarphot/core.py
88b5760 
Accepted this suggestion by Matt on PR#97.

Co-authored-by: Matt Craig <[email protected]>
@JuanCab @mwcraig
Update stellarphot/core.py
047e044 
Accepted this suggestion by Matt on PR#97.

Co-authored-by: Matt Craig <[email protected]>
@JuanCab @mwcraig
Update stellarphot/core.py
b4a0774 
Accepted this suggestion by Matt on PR#97.

Co-authored-by: Matt Craig <[email protected]>
@JuanCab @mwcraig
Update stellarphot/core.py
da8fe2c 
Accepted this suggestion by Matt on PR#97.

Co-authored-by: Matt Craig <[email protected]>
@JuanCab @mwcraig
Update stellarphot/differential_photometry/magnitude_transforms.py
4e64db0 
Accepted this suggestion by Matt on PR#97.

Co-authored-by: Matt Craig <[email protected]>
@JuanCab @mwcraig
Update stellarphot/io/aij.py
96492e2 
Accepted this suggestion by Matt on PR#97.

Co-authored-by: Matt Craig <[email protected]>
@JuanCab @mwcraig
Update stellarphot/io/aij.py
b0ddb65 
Accepted this suggestion by Matt on PR#97.

Co-authored-by: Matt Craig <[email protected]>
@JuanCab @mwcraig
Update stellarphot/io/tess.py
3492cac 
Accepted this suggestion by Matt on PR#97.

Co-authored-by: Matt Craig <[email protected]>
@JuanCab @mwcraig
Update stellarphot/io/tess.py
14cad6d 
Accepted this suggestion by Matt on PR#97.

Co-authored-by: Matt Craig <[email protected]>
@JuanCab @mwcraig
Update stellarphot/io/tess.py
4a60757 
Accepted this suggestion by Matt on PR#97.

Co-authored-by: Matt Craig <[email protected]>
@JuanCab @mwcraig
Update stellarphot/io/tess.py
6698bd9 
Accepted this suggestion by Matt on PR#97.

Co-authored-by: Matt Craig <[email protected]>
mwcraig
mwcraig reviewed Jun 12, 2023
View reviewed changes
Copy link
Contributor

@mwcraig mwcraig left a comment

Choose a reason for hiding this comment

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

Found one more little thing in looking over the changes

stellarphot/io/tess.py Outdated Show resolved Hide resolved
@JuanCab @mwcraig
Update stellarphot/io/tess.py
d230388 
Accepted this suggestion by Matt on PR#97.

Co-authored-by: Matt Craig <[email protected]>
@mwcraig
Copy link
Contributor

mwcraig commented Jun 14, 2023

Thanks again, @JuanCab

@mwcraig mwcraig merged commit 8f8e5f7 into feder-observatory:main Jun 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mwcraig mwcraig mwcraig approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@JuanCab @mwcraig