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

Convert general submodule docstrings to numpydoc #53

Merged
merged 2 commits into from
Mar 15, 2024
Merged

Conversation

K-Meech
Copy link
Contributor

@K-Meech K-Meech commented Mar 15, 2024

Description

What is this PR

  • Bug fix
  • Addition of a new feature
  • Other

Why is this PR needed?
Docstrings in the general sub-module are currently in reST style.

What does this PR do?
Converts all docstrings in the general sub-module to numpdoc style.

References

For #19

How has this PR been tested?

All tests run locally.

Is this a breaking change?

No.

Does this PR require an update to the documentation?

No - docstrings have been updated.

Checklist:

  • The code has been tested locally
  • Tests have been added to cover all new functionality (unit & integration)
  • The documentation has been updated to reflect any changes
  • The code has been formatted with pre-commit

@K-Meech K-Meech requested a review from a team March 15, 2024 14:21
Copy link
Contributor

@willGraham01 willGraham01 left a comment

Choose a reason for hiding this comment

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

👍

Though the choice of implementation for one of the functions confuses me... we seem to have a severe aversion to the number 0? 😅

Returns True if a number is even
:param num:
:return:
Returns True if a number is even.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Returns True if a number is even.
Returns True if the non-zero input number is even.

I have no idea why we throw an error if the input is zero (since 0 is a perfectly fine even number) but I presume that this function will be used further up the dependency chain in which this case might be important?

Either way, it probably needs a Raises section added too if we're being docstring-precise.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That is weird - not sure why this is needed! I added your suggestion + a Raises section to indicate zero isn't accepted

@K-Meech K-Meech merged commit 3411a1b into main Mar 15, 2024
10 checks passed
@K-Meech K-Meech deleted the general_docstrings branch March 15, 2024 17:09
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.

2 participants