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

clean up docstring formatting and type hints #16

Merged
merged 6 commits into from
Aug 17, 2023

Conversation

aidanjgriffiths
Copy link
Collaborator

@aidanjgriffiths aidanjgriffiths commented Jun 9, 2023

Tasks

This PR also addresses the relative imports in the scores.probability module

@aidanjgriffiths aidanjgriffiths linked an issue Jun 9, 2023 that may be closed by this pull request
2 tasks
@aidanjgriffiths aidanjgriffiths marked this pull request as draft June 9, 2023 06:53
@aidanjgriffiths aidanjgriffiths changed the title Draft: clean up docstring formatting and type hints clean up docstring formatting and type hints Aug 11, 2023
@aidanjgriffiths aidanjgriffiths marked this pull request as ready for review August 11, 2023 00:31
@aidanjgriffiths
Copy link
Collaborator Author

aidanjgriffiths commented Aug 11, 2023

@nicholasloveday and @tennlee, this MR has been rebased with main and is ready to review.
I will copy spin out the remaining tasks into seperate MRs since I don't really have the capacity to address them right now.

src/scores/continuous.py Outdated Show resolved Hide resolved
src/scores/continuous.py Outdated Show resolved Hide resolved
@tennlee
Copy link
Collaborator

tennlee commented Aug 11, 2023

Thank you very much for all this work. I think we should talk about splitting up the MR into smaller parts, as I think it might take a while to work through the discussion about the right constraints for the type hinting, whereas the other parts of the MR could go ahead without much discussion.

@nicholasloveday
Copy link
Collaborator

@aidanjgriffiths some functions specify the type in the docstring (e.g., mae), while other functions don't (e.g., mse).
Can you make this consistent? I think the type is not needed in the docstring if you are using typehints.

@aidanjgriffiths aidanjgriffiths force-pushed the 1-clean-up-docstring-formatting branch 2 times, most recently from f5256b1 to 28bd6e2 Compare August 15, 2023 06:37
Copy link
Collaborator

@tennlee tennlee left a comment

Choose a reason for hiding this comment

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

Thanks very much for this work! I've commented on a few parts which need a little more tweaking before it's done.

src/scores/continuous.py Outdated Show resolved Hide resolved
src/scores/continuous.py Show resolved Hide resolved
src/scores/continuous.py Outdated Show resolved Hide resolved
src/scores/continuous.py Outdated Show resolved Hide resolved
src/scores/continuous.py Outdated Show resolved Hide resolved
src/scores/continuous.py Outdated Show resolved Hide resolved
src/scores/probability/checks.py Outdated Show resolved Hide resolved
src/scores/probability/crps_impl.py Outdated Show resolved Hide resolved
src/scores/utils.py Outdated Show resolved Hide resolved
src/scores/continuous.py Outdated Show resolved Hide resolved
src/scores/continuous.py Outdated Show resolved Hide resolved
src/scores/continuous.py Outdated Show resolved Hide resolved
src/scores/continuous.py Outdated Show resolved Hide resolved
src/scores/continuous.py Outdated Show resolved Hide resolved
src/scores/continuous.py Outdated Show resolved Hide resolved
@tennlee
Copy link
Collaborator

tennlee commented Aug 17, 2023

I have directly discussed readiness of this merge with the author, so I am manually overriding the change request status. Thanks very much for the work!

@tennlee tennlee merged commit 68875f1 into main Aug 17, 2023
4 checks passed
@tennlee tennlee deleted the 1-clean-up-docstring-formatting branch August 17, 2023 06:57
aidanjgriffiths added a commit that referenced this pull request Aug 21, 2023
* docs: align docstrings with napoleon (google) standards
* refactor: replace relative imports with pkg level imports
* docs: updating contributing notes to include napoleon docstring style
* docs: review amendments to docstrings/types
* docs: update coords_increasing check docstring
* docs: gather dims union types and mse types

---------

Signed-off-by: Aidan Griffiths <[email protected]>
Co-authored-by: agriffit <[email protected]>
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.

Clean up docstring formatting
3 participants