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

numpydoc style(signac/contrib/errors.py, signac/contrib/filesystems.py, signac/contrib/filterparser.py, signac/contrib/hashing.py) #322

Merged
merged 20 commits into from
May 7, 2020

Conversation

ac-optimus
Copy link
Contributor

Description

issue #315

Motivation and Context

Types of Changes

  • Documentation update
  • Bug fix
  • New feature
  • Breaking change1

1The change breaks (or has the potential to break) existing functionality.

Checklist:

If necessary:

  • I have updated the API documentation as part of the package doc-strings.
  • I have created a separate pull request to update the framework documentation on signac-docs and linked it here.
  • I have updated the changelog and added all related issue and pull request numbers for future reference (if applicable). See example below.

Change of doc string to numpydoc in schema.py

@ac-optimus ac-optimus requested review from a team as code owners April 8, 2020 11:12
@codecov
Copy link

codecov bot commented Apr 8, 2020

Codecov Report

Merging #322 into numpy_docs will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff             @@
##           numpy_docs     #322   +/-   ##
===========================================
  Coverage       76.20%   76.20%           
===========================================
  Files              43       43           
  Lines            7080     7082    +2     
===========================================
+ Hits             5395     5397    +2     
  Misses           1685     1685           
Impacted Files Coverage Δ
signac/common/errors.py 76.92% <100.00%> (+1.92%) ⬆️
signac/contrib/errors.py 94.11% <100.00%> (-0.62%) ⬇️
signac/contrib/filterparse.py 84.12% <100.00%> (+0.25%) ⬆️
signac/contrib/hashing.py 100.00% <100.00%> (ø)
signac/core/errors.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ed69d02...ba92ecb. Read the comment docs.

@vyasr vyasr requested review from a team, yuanzhou0827 and tcmoore3 and removed request for a team April 8, 2020 16:53
Copy link
Member

@bdice bdice left a comment

Choose a reason for hiding this comment

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

@ac-optimus Here are some initial suggestions. This will need further iterations. Please read over the NumPy docstring style guide carefully. I haven't had a chance to read it in full, but it will probably answer some of my questions in this review. Also, it will help us be consistent if we adhere to those rules. https://numpydoc.readthedocs.io/en/latest/format.html

signac/contrib/filesystems.py Outdated Show resolved Hide resolved
signac/contrib/filterparse.py Outdated Show resolved Hide resolved
signac/contrib/filterparse.py Outdated Show resolved Hide resolved
signac/contrib/filterparse.py Outdated Show resolved Hide resolved
signac/contrib/filterparse.py Outdated Show resolved Hide resolved
signac/contrib/filterparse.py Outdated Show resolved Hide resolved
signac/contrib/filterparse.py Outdated Show resolved Hide resolved
signac/contrib/filterparse.py Outdated Show resolved Hide resolved
signac/contrib/hashing.py Outdated Show resolved Hide resolved
signac/contrib/hashing.py Outdated Show resolved Hide resolved
@bdice
Copy link
Member

bdice commented Apr 9, 2020

@ac-optimus I edited the setup.cfg file and a few of the files that this PR is meant to update. Take a look at those edits, since we'll need to make similar changes in the other docstyle pull requests. The changes in setup.cfg will probably conflict for all of these pull requests, and will need to be resolved as the PRs are merged into the numpy_docs branch.

I left filesystems.py off of the list of pydocstyle files in setup.cfg, since we shouldn't bother updating docs for all the deprecated features that we plan to remove soon.

@bdice
Copy link
Member

bdice commented Apr 9, 2020

@ac-optimus I had a typo in the name of filterparse.py. I pushed that change to setup.cfg but the docstrings in that file need to be updated. You can see the errors by installing and running pydocstyle in the repository root.

@ac-optimus
Copy link
Contributor Author

@bdice I have made the changes. Thank you for pointing me to pydocstyle.
Will modify setup.cfg accordingly in other PRs as well.

@ac-optimus ac-optimus requested a review from bdice April 10, 2020 11:22
signac/contrib/hashing.py Outdated Show resolved Hide resolved
@ac-optimus ac-optimus requested a review from bdice April 16, 2020 20:20
@bdice bdice mentioned this pull request Apr 18, 2020
12 tasks
@bdice bdice added this to the v1.5.0 milestone Apr 24, 2020
Copy link
Member

@tcmoore3 tcmoore3 left a comment

Choose a reason for hiding this comment

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

LGTM

signac/contrib/filterparse.py Outdated Show resolved Hide resolved
signac/contrib/filterparse.py Show resolved Hide resolved
@mikemhenry mikemhenry added the doc-style Update docs to numpy doc style label May 5, 2020
Copy link

@yuanzhou0827 yuanzhou0827 left a comment

Choose a reason for hiding this comment

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

All changes look good to me except the added period mentioned by Tim.

Copy link
Member

@bdice bdice left a comment

Choose a reason for hiding this comment

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

I am going to apply my own change requests and merge this PR.

signac/contrib/filterparse.py Outdated Show resolved Hide resolved
signac/contrib/filterparse.py Show resolved Hide resolved
signac/contrib/filterparse.py Outdated Show resolved Hide resolved
signac/contrib/filterparse.py Outdated Show resolved Hide resolved

Returns
-------

Copy link
Member

@bdice bdice May 7, 2020

Choose a reason for hiding this comment

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

Unfortunately that example is for a slightly different case. That would apply if _cast returned two values. This function doesn't have a defined return type, so it should claim its return type is object. I will make a suggestion above.

signac/contrib/filterparse.py Outdated Show resolved Hide resolved
signac/contrib/filterparse.py Outdated Show resolved Hide resolved
signac/contrib/filterparse.py Show resolved Hide resolved
signac/contrib/filterparse.py Outdated Show resolved Hide resolved
@bdice bdice merged commit 3e2417c into glotzerlab:numpy_docs May 7, 2020
mikemhenry pushed a commit that referenced this pull request Jul 22, 2020
* numpydoc style(signac/contrib/schema.py) (#318)

* numpydoc style(signac/contrib/project.py) (#320)

* numpydoc style(signac/contrib/job.py)  (#319)

* numpydoc style(signac/contrib/linked_view.py, signac/contrib/utility.py, signac/core/utility.py) (#326)

* numpydoc style(signac/contrib/errors.py, signac/contrib/filesystems.py, signac/contrib/filterparser.py, signac/contrib/hashing.py) (#322)

* numpydoc style (signac/contrib/collection.py) (#329)

* Make core submodule adhere to numpydoc conventions. (#339)

* add a docstring for public module (#337)

* numpydoc style (signac/contrib/import_export.py) (#333)

* Apply NumPy docstyle to remaining files in "signac" root. (#343)

* Apply fixes from minirst. (#356)

* Enforce pydocstyle in core, common, contrib. (#354)

Co-authored-by: Bradley Dice <[email protected]>
Co-authored-by: Alyssa Travitz <[email protected]>
Co-authored-by: Abhavya Chandra <[email protected]>
Co-authored-by: vyasr <[email protected]>
Co-authored-by: Pengji <[email protected]>
Co-authored-by: Mike Henry <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc-style Update docs to numpy doc style
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants