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

Additional code formatting/linting without modernizing Python syntax #1598

Merged
merged 4 commits into from
Apr 29, 2022

Conversation

zaneselvans
Copy link
Member

@zaneselvans zaneselvans commented Apr 29, 2022

This PR adds a few more pre-commit hooks that check the code and especially docstrings / docs for various syntax and formatting issues, including:

  • using only absolute imports
  • removing unnecessary f-strings
  • Enabling docstring-convention = google within flake8 which enforces a bunch of RST formatting in docstrings that we were missing before (this should clean up our documentation output a bit).
  • Added the rstcheck linter to the docs build with Tox, in addition to running it in the pre-commit hooks.

I also tried to add some python syntax updates using these formatters / pre-commit hooks:

However, it turns out there are some incompatibilities between Pydantic and the use of

from __futures__ import annotations

The use of __futures__ is required in order to make new style annotations work with Python 3.8 and 3.9. In general they work great though, and if/when we want to leave behind Python 3.8 and 3.9 we can automatically update all of the code to use the newer versions of type annotations and other syntax changes.

This commit touches a ton of files, but almost exclusively changes docstrings, not code, so hopefully it won't create any headaches for merging.

There are a handful of functions which are missing descriptions for their arguments, which results in a D417 error. I've deactivated these with

# noqa: D417

but we should make sure that the arguments get documented.

@zaneselvans zaneselvans mentioned this pull request Apr 29, 2022
@codecov
Copy link

codecov bot commented Apr 29, 2022

Codecov Report

Merging #1598 (492dd6d) into dev (6a7ae59) will increase coverage by 0.0%.
The diff coverage is 98.8%.

@@          Coverage Diff          @@
##             dev   #1598   +/-   ##
=====================================
  Coverage   84.1%   84.2%           
=====================================
  Files         65      65           
  Lines       7227    7228    +1     
=====================================
+ Hits        6085    6086    +1     
  Misses      1142    1142           
Impacted Files Coverage Δ
src/pudl/analysis/epa_crosswalk.py 96.4% <ø> (ø)
src/pudl/analysis/mcoe.py 92.4% <ø> (ø)
src/pudl/analysis/service_territory.py 29.0% <ø> (ø)
src/pudl/analysis/spatial.py 93.6% <ø> (ø)
src/pudl/analysis/timeseries_cleaning.py 88.6% <ø> (ø)
src/pudl/cli.py 67.5% <ø> (ø)
src/pudl/convert/censusdp1tract_to_sqlite.py 91.6% <ø> (ø)
src/pudl/convert/epacems_to_parquet.py 74.1% <ø> (ø)
src/pudl/convert/ferc1_to_sqlite.py 62.1% <ø> (ø)
src/pudl/convert/metadata_to_rst.py 69.5% <ø> (ø)
... and 40 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 6a7ae59...492dd6d. Read the comment docs.

@zaneselvans zaneselvans merged commit a51ef8b into dev Apr 29, 2022
@zaneselvans zaneselvans deleted the paleopython branch April 29, 2022 15:42
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