Add Doc Linting to CI

[mikemhenry]

Add automated doc string checking to CI

Description

We are using pydocstyle to check that our document strings are in the numpy docstring style. The configuration for pydocstyle is in setup.cfg. For now I'm keeping things rather basic, but we might find it worth printing more detailed information once we get everything passing. I also chose signac/core/jsondict.py as the first file for testing this, feel free to suggest another file if you think there is a better candidate.

Before we fix the issues with this file's document strings, I want to make sure CI will throw an error.

@jennyfothergill is co-authoring this PR.

Motivation and Context

See this issue for more detail:
glotzerlab/signac-docs#64

Types of Changes

• Documentation update
• Bug fix
• New feature
• Breaking change¹

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

Checklist:

• I am familiar with the Contributing Guidelines.
• I agree with the terms of the Contributor Agreement.
• My name is on the list of contributors.
• My code follows the code style guideline of this project.
• The changes introduced by this pull request are covered by existing or newly introduced tests.

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.

Example for a changelog entry:
Add docstring linting to CI
Fix docstring formatting in signac/core/jsondict.py

[mikemhenry]

Looks like CI is working how I would expect and fails due to some formatting errors.

[vyasr]

@mikemhenry what sort of input will it take to get this out of draft? What decisions need to be made to move this PR forward? Deciding whether pydocstyle is the right tool?

[vyasr]

@mikemhenry how are things here? If possible, it would be nice to make progress with this PR so that we can have some of our new contributors look into the downstream issues.

[mikemhenry]

Next steps to get this out of draft:

1. Refactor our CI a bit to separate out code linting, doc linting, and dependency install.
2. Confirm the CI fails for doc string testing
3. Fix the file
   Then we can merge 🎉

[mikemhenry]

@bdice I've made some changes to the CI, looking at the output it looks good to me, was that what you were thinking?

[bdice]

@mikemhenry Looks swiggity sweet to me. With that, (1) and (2) are done. Then we just fix that file (3) and it will be ready to merge. After that we can open issues to improve doc style for each submodule (signac.contrib, signac.core, etc.) and explicitly exclude the deprecated bits like signac.db.

[mikemhenry]

Okay it should pass the linting now, but I put in some place holder """Doc string here.""" for the classes and methods that were missing them. I would be happy to take suggestions on what the doc strings should be @bdice @vyasr @csadorf

[codecov]

Codecov Report

Merging #294 into master will decrease coverage by 11.02%.
The diff coverage is 100%.

@@             Coverage Diff             @@
##           master     #294       +/-   ##
===========================================
- Coverage   76.18%   65.15%   -11.03%     
===========================================
  Files          43       40        -3     
  Lines        7076     5700     -1376     
===========================================
- Hits         5391     3714     -1677     
- Misses       1685     1986      +301

Impacted Files	Coverage Δ
signac/core/jsondict.py	95.34% <100%> (ø)	⬆️
signac/db/database.py	0% <0%> (-100%)	⬇️
signac/__main__.py	0% <0%> (-80%)	⬇️
signac/common/crypt.py	0% <0%> (-76.75%)	⬇️
signac/common/host.py	0% <0%> (-46.43%)	⬇️
signac/common/validate.py	39.13% <0%> (-39.14%)	⬇️
signac/common/connection.py	0% <0%> (-38.28%)	⬇️
signac/common/config.py	70.37% <0%> (-15.75%)	⬇️
signac/contrib/filesystems.py	33.96% <0%> (-12.27%)	⬇️
signac/contrib/utility.py	51.49% <0%> (-7.19%)	⬇️
... and 12 more

---

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 232d018...554bc6c. Read the comment docs.

[csadorf]

Okay it should pass the linting now, but I put in some place holder """Doc string here.""" for the classes and methods that were missing them. I would be happy to take suggestions on what the doc strings should be @bdice @vyasr @csadorf

Can you create a list linking to those placeholders and then we can work through them?

[mikemhenry]

@csadorf Done!

BufferException

• __str__

JSONDict

• buffered

BufferedSyncedAttrDict

• BufferedSyncedAttrDict

• load

• save

• flush

[vyasr]

As per PyCQA/pydocstyle#60, I'm in favor of not requiring docstrings for magic methods. pydocstyle supports suppressing this warning, and I think that makes sense since the behavior of most magic methods should be obvious in the majority of cases.

• JSONDict.buffered: A context manager for buffering reads from and writes to this JSONDict that flushes all changes when the context is exited.
• BufferedSyncedAttrDict: A buffered :class:~.SyncedAttrDict that saves all changes in memory but does not write them to file until :meth:~.flush is called.
• BufferedSyncedAttrDict.flush: Save buffered changes to the underlying file.

For load and save, I'd prefer to add the documentation to the parent class (_SyncedDict). Does pycodestyle correctly detect inheritance of docstrings? Most Python tools (like Sphinx and help) support this, so I would hope pycodestyle would as well.

@csadorf @bdice thoughts?

[mikemhenry]

I'm fine with ignoring magic methods as a default. If during a code review there is a magic method that seems confusing, that would be the time to make sure we get it documented. RE docstring inheritance, I will check to see if pydocestyle supports it. Since right now I only whitelist one file, that might interfere with its abilities.

[mikemhenry]

I haven't found a tool that will be able to lint docstrings and understand inheriting them from different classes and it is a know issue: PyCQA/pydocstyle#309
I decided to go with the noqa method, but we can switch to using a decorator or something else.

[mikemhenry]

I had to manually re-run some tests but everything passes now and this PR is ready for review!

[bdice]

@mikemhenry I pushed one set of minor edits and have one comment. Otherwise approved.

[csadorf]

The doc-string for the __str__ method still needs to be removed, and I've made two suggestions for improvement, but otherwise this looks good to me. Thx a lot!

[bdice]

I am going to merge this since it has 2 approvals - I applied a couple of small changes at the end to ignore D105. To ignore D105, I had to disable the "numpy" convention. I reviewed the rules that are ignored by the numpy convention and I removed a couple of those ignore rules (so we're slightly "stricter"). See here for info: http://www.pydocstyle.org/en/latest/error_codes.html#default-conventions

[mikemhenry]

Sweet, thanks so much for taking those lest steps!