pytorch / pytorch/vision

Split autoformatters and linters into different workflows and CI jobs

Open
#5,167 5 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

code quality module: ci needs discussion
Dominant language
Python
Stars
17.9k
Forks
7.3k
Avg merge
1d 15h
Merged PRs (30d)
13

Description

Status quo

Currently we mix formatters and linters with pre-commit:

formatters

https://github.com/pytorch/vision/blob/578c1546c328c68e601bfd2e2309ed175ee343d9/.pre-commit-config.yaml#L9-L11

https://github.com/pytorch/vision/blob/578c1546c328c68e601bfd2e2309ed175ee343d9/.pre-commit-config.yaml#L23-L26

linters

https://github.com/pytorch/vision/blob/578c1546c328c68e601bfd2e2309ed175ee343d9/.pre-commit-config.yaml#L5-L8

https://github.com/pytorch/vision/blob/578c1546c328c68e601bfd2e2309ed175ee343d9/.pre-commit-config.yaml#L31-L32

https://github.com/pytorch/vision/blob/578c1546c328c68e601bfd2e2309ed175ee343d9/.pre-commit-config.yaml#L37

In addition we have a separate CI job that lints with mypy

https://github.com/pytorch/vision/blob/578c1546c328c68e601bfd2e2309ed175ee343d9/.circleci/config.yml#L298

Proposal

I propose we change the rationale from the two CI jobs from pre-commit and mypy to code format and lint. That means, we would only keep autoformatters in our pre-commit configuration and move all linters to what is currently the mypy job.

Pros
  • We could use pre-commit as "single source of truth" for formatting code. Currently they mentioned as "purely optional" in our contribution guide. Since pre-commit supports running the hooks manually, we can simply treat it as our way to bundle all autoformatters while the user does not need to know or care what exactly is run.
  • We could simply add new autoformatters if they are available through pre-commit, e.g. #5158. If anyone applies code formatting through pre-commit anyway, that won't break any workflow.
Cons
  • We would loose the ability to run linters in a bundled manner. I don't think this is a strong con, since linters such as flake8 or pydocstyle trigger very seldom anyway due to the auto-formatting. Plus, we already need to run mypy separately.

cc @seemethere

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by comparing the formatter and linter hooks in .pre-commit-config.yaml with the mypy job and related CI configuration in .circleci/config.yml. Separate the formatting and linting responsibilities so the resulting CI jobs and pre-commit setup match the proposal, then verify the contribution guidance reflects the new roles.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
build-system, ci-cd, tooling
Issue type
Refactor
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.