Skip to content

Enable UFMT on all files in PyTorch #123062

New issue
New issue
Open
Open
Enable UFMT on all files in PyTorch#123062
Labels
actionablebetter-engineeringRelatively self-contained tasks for better engineering contributorsmodule: lintIssues related to our Python/C++ lint rules (run by Travis)triagedThis issue has been looked at a team member, and triaged and prioritized into an appropriate module
@ezyang

Description

@ezyang
ezyang
opened on Mar 31, 2024

🐛 Describe the bug

Right now, around 1.5k files are grandfathered in as "not UFMT'ed" in the PyTorch codebase. Let's make them all formatted!

To format a file, remove its name from exclude_patterns of UFMT section in .lintrunner.toml. Then, run lintrunner -a --take UFMT --all-files. You'll end up with a pull request like this: #123061

For ease of review, the UFMT diff should be 100% generatable from the .lintrunner.toml change. In particular, if you are code reviewing a UFMT change from an untrusted source, clear all changes except the .lintrunner.toml and manually rerun lintrunner to regenerate the changes. To verify that there are no changes, a maintainer can use this script: You may need to make fixup changes. This means maintainers MUST verify the additional changes on top of lint. This can be done with this script:

#!/bin/bash
set -ex
gh pr checkout $1
REV=$(git rev-parse HEAD)
git fetch origin
git checkout $(git merge-base origin/main HEAD)
git checkout $REV .lintrunner.toml
lintrunner init
lintrunner -a --take UFMT --all-files
git diff $REV

In some cases, preparatory work needs to be done. Here are some known problems:

  • torch/cuda/_memory_viz.py - mypy type: ignore annotations get moved in such a way that an ignore is no longer applied to the correct line
  • test/functorch/test_control_flow.py - flake8: reformatting moves flake8 ignore to the wrong line, and now a multiline string no longer has flake8 suppressed
  • torch/masked/ - the import reordering causes an import cycle
  • ... probably more ...

These problems should be resolved in preparatory commits BEFORE the UFMT commit, for ease of review.

How to split the work? I decided to bundle things up by directory, so long as there were not too many changes in one directory that necessitated splitting it up. With https://gist.github.com/ezyang/6c10a63d5795cbd7787beeea94497948 I did a sweep to find an optimal threshold:

image

13k seems like a good splitting point. Here is the worklist:

This list does not cover torch/testing/_internal/common_methods_invocations.py, which has a whopping 35341 lines of changes and will likely have to be handled in some other way (e.g., splitting it first).

To claim an entry, put your name (and eventually PR) next to it by editing this post. Feel free to bundle multiple small but related changes together if necessary.

Versions

main

Metadata

Metadata

Assignees

No one assigned

    Labels

    actionablebetter-engineeringRelatively self-contained tasks for better engineering contributorsmodule: lintIssues related to our Python/C++ lint rules (run by Travis)triagedThis issue has been looked at a team member, and triaged and prioritized into an appropriate module

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions