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

docs: specify empty docstring fields #36

Merged
merged 8 commits into from
Jul 21, 2023
Merged

docs: specify empty docstring fields #36

merged 8 commits into from
Jul 21, 2023

Conversation

benwandrew
Copy link
Contributor

@benwandrew benwandrew commented Jul 17, 2023 •
edited
Loading

Description

add descriptions to empty fields in docstring where appropriate

Type of change

  • docs: Documentation only changes

hollandjg
hollandjg reviewed Jul 18, 2023
View reviewed changes
Copy link
Member

@hollandjg hollandjg left a comment

Choose a reason for hiding this comment

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

Hey Ben, thanks for going through these! I'm not sure what the best description is for the arguments of the wrapper function, but I've made some suggestions which are more focused on the properties of the argument. Do those also make sense for you?

src/autora/state/delta.py Outdated Show resolved Hide resolved
src/autora/state/delta.py Outdated Show resolved Hide resolved
@benwandrew benwandrew changed the title docs: specify docstring fields in wrap_to_use_state docs: specify empty docstring fields Jul 18, 2023
benwandrew and others added 2 commits July 18, 2023 10:35
@benwandrew @hollandjg
Update src/autora/state/delta.py
626990a 
Co-authored-by: John Gerrard Holland <[email protected]>
@benwandrew @hollandjg
Update src/autora/state/delta.py
efc2929 
Co-authored-by: John Gerrard Holland <[email protected]>
@benwandrew
Copy link
Contributor Author

Hey Ben, thanks for going through these! I'm not sure what the best description is for the arguments of the wrapper function, but I've made some suggestions which are more focused on the properties of the argument. Do those also make sense for you?

yep, those are both more on the mark. thank you!

hollandjg
hollandjg approved these changes Jul 21, 2023
View reviewed changes
Copy link
Member

@hollandjg hollandjg left a comment

Choose a reason for hiding this comment

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

looks good to me!

@benwandrew benwandrew added this pull request to the merge queue Jul 21, 2023
Merged via the queue into main with commit 1a8910d Jul 21, 2023
13 checks passed
@benwandrew benwandrew deleted the docs/specify-docstring-fields branch July 21, 2023 15:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hollandjg hollandjg hollandjg approved these changes

@younesStrittmatter younesStrittmatter Awaiting requested review from younesStrittmatter

@musslick musslick Awaiting requested review from musslick

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
@benwandrew @hollandjg