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

Only Modified Base.py #21

Conversation

ranbir7
Copy link
Contributor

@ranbir7 ranbir7 commented Feb 6, 2023

Please Tell me if this is the way I should use docstrings...

Copy link
Member

@ConnorStoneAstro ConnorStoneAstro left a comment

Choose a reason for hiding this comment

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

This looks great! just two minor points on the data type of the lambda_history variable

""" Returns the value of lambda (regularization strength) at which minimum chi^2 loss was achieved.

Returns:
float: Value of lambda at which minimum chi^2 loss was achieved.
Copy link
Member

Choose a reason for hiding this comment

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

this is right except the return type is ndarray, not float

iteration (int): The current iteration number.
save_steps (Union[None, int]): The frequency at which to save intermediate results.
relative_tolerance (float): The relative tolerance for the optimization.
lambda_history (List[float]): A list of the optimization steps.
Copy link
Member

Choose a reason for hiding this comment

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

The type here is List[ndarray], not list[float]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay I'll be resolving it right now
Thanks
So I have to do it for all codes in autoprof dir?

Copy link
Member

Choose a reason for hiding this comment

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

for now you can just change float -> ndarray in the line above. Eventually it would be great to do docstrings for every function, but that can be done in a future PR.

@ConnorStoneAstro
Copy link
Member

Hi ranbir7, those updates to the docs look great! Thanks so much for helping out with the development, I really appreciate it! I have noted two points where there is a slight mistake in the data type indicated for the lambda_history variable. This is my first time running a PR from an external user, is it possible for you to make those minor changes? If so that would be great :) Then I'll approve the merge request and it will be part of the main branch! This is very exciting!

@ConnorStoneAstro ConnorStoneAstro linked an issue Feb 7, 2023 that may be closed by this pull request
@ranbir7
Copy link
Contributor Author

ranbir7 commented Feb 7, 2023

yes I'll be resolving those changes

Copy link
Member

@ConnorStoneAstro ConnorStoneAstro left a comment

Choose a reason for hiding this comment

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

The new docs look great! Thanks so much for helping!

@ConnorStoneAstro ConnorStoneAstro merged commit 40901c4 into Autostronomy:main Feb 7, 2023
@ranbir7 ranbir7 deleted the ranbir7/convert-docstrings-to-google-18 branch February 7, 2023 05:23
@ranbir7 ranbir7 restored the ranbir7/convert-docstrings-to-google-18 branch February 7, 2023 05:23
@ranbir7
Copy link
Contributor Author

ranbir7 commented Feb 7, 2023

I'll be doing for the rest .py files in the fit dir
Hope it helps u again
and please let me know if there's any mistake
I'll be always helping 😁

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

Successfully merging this pull request may close these issues.

Convert docstrings to google format
2 participants