TRI-ML / dgp

ML Dataset Governance Policy for Autonomous Vehicle Datasets
https://tri-ml.github.io/dgp/
MIT License
93 stars 63 forks source link

feat: fix latent pylint errors in many source files #115

Closed tk-woven closed 2 years ago

tk-woven commented 2 years ago

Description

This is toward #114. We'd like to replace the many-entrypoint linting with the single-entrypoint pre-commit tool. When run in CI, we'll run pre-commit against all files for convenience. So, I ran pylint against all files in DGP and found some latent issues in master. This MR addresses the issues. The issues are almost entirely documentation issues, so the bulk of this MR is docs changes.

Note one quirk. According to the numpydoc style guide, we should be able to document parameters as

Parameters
----------
name: type, default: value
    Description.

if name gets a default value such as in f(name: str = "asdf"). However, pylint with the numpydoc plugin does not seem to recognize the default: value syntax when placed inline with the type documentation. To get around this, we use the canonical optional annotation and move the default value annotation into the parameter description.

Besides these docs changes, this does introduce some new code for calls to open. Pylint is very strict and requires we provide an encoding, so we just explicitly specify what will be used as the default in the end anyway, locale.getpreferredencoding(). Alternatively, we could suppress this pylint warning. This is only done for text-mode (not binary-mode) files.

Lastly, there were a few places where I simply couldn't intuit the types reasonably. In these cases (and in test code), I simply asked pylint to ignore missing docs.

Checks

I built the docs locally after this change. There are very many Sphinx warnings about unexpected section titles (for all sections: Parameters, Returns, etc.) in documentation of callables. These issues appear to exist on master already.

I reviewed the built documentation (and PDF, a la rinoh), and they still appear conveniently human-readable to me.


This change is Reviewable

tk-woven commented 2 years ago

Thanks!