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

flake8 and mypy errors #7200

Open
mpearce25 opened this issue Feb 9, 2023 · 4 comments
Open

flake8 and mypy errors #7200

mpearce25 opened this issue Feb 9, 2023 · 4 comments

Comments

@mpearce25
Copy link
Contributor

mpearce25 commented Feb 9, 2023

🐛 Describe the bug

I'm planning to contribute type annotations to the library as discussed and tracked in #2025. However, I encountered multiple flake8 and mypy errors on main as of 2cd25c1a05012a3720a87f20cec436811fadeedd. I'd like to fix these before adding additional type annotations. If the proposed fixes below look good I can make a PR.


Running

mypy --config-file mypy.ini yields

torchvision/prototype/datapoints/_datapoint.py:8: error: Module "torch._C" has no attribute "DisableTorchFunctionSubclass"  [attr-defined]
    from torch._C import DisableTorchFunctionSubclass
    ^
torchvision/prototype/transforms/_augment.py:137: error: Unused "type: ignore" comment
            return dict(lam=float(self._dist.sample(())))  # type: ignore[arg-type]
            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
torchvision/prototype/transforms/_augment.py:159: error: Unused "type: ignore" comment
            lam = float(self._dist.sample(()))  # type: ignore[arg-type]
            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Found 3 errors in 2 files (checked 227 source files)

To address these I would

  • Remove the two extra type ignores
  • Add # type: ignore[attr-defined] after from torch._C import DisableTorchFunctionSubclass

Running flake8 yields

./test/test_ops.py:6:1: F401 'typing.Tuple' imported but unused
./torchvision/models/detection/generalized_rcnn.py:7:1: F401 'typing.Dict' imported but unused
./torchvision/models/detection/generalized_rcnn.py:7:1: F401 'typing.Optional' imported but unused
./torchvision/models/detection/generalized_rcnn.py:7:1: F401 'typing.Union' imported but unused
./torchvision/models/detection/generalized_rcnn.py:10:1: F401 'torch.Tensor' imported but unused
./torchvision/models/detection/roi_heads.py:1:1: F401 'typing.Optional' imported but unused
./torchvision/models/detection/roi_heads.py:1:1: F401 'typing.Tuple' imported but unused
./torchvision/models/detection/roi_heads.py:6:1: F401 'torch.Tensor' imported but unused

To address these I would

  • Remove the unused type imports. They only seemingly get used in docstring type hints. However, removing the import fixes the flake8 errors and doesn't cause issues with ufmt in my testing. example of current docstring type hint that likely led to these imports being added
    # type: (Dict[str, Tensor], List[Dict[str, Tensor]]) -> Union[Dict[str, Tensor], List[Dict[str, Tensor]]]

Versions

Collecting environment information...
PyTorch version: 1.13.1
Is debug build: False
CUDA used to build PyTorch: None
ROCM used to build PyTorch: N/A

OS: macOS 13.1 (x86_64)
GCC version: Could not collect
Clang version: 14.0.0 (clang-1400.0.29.202)
CMake version: Could not collect
Libc version: N/A

Python version: 3.9.16 | packaged by conda-forge | (main, Feb  1 2023, 21:42:20)  [Clang 14.0.6 ] (64-bit runtime)
Python platform: macOS-13.1-x86_64-i386-64bit
Is CUDA available: False
CUDA runtime version: No CUDA
CUDA_MODULE_LOADING set to: N/A
GPU models and configuration: No CUDA
Nvidia driver version: No CUDA
cuDNN version: No CUDA
HIP runtime version: N/A
MIOpen runtime version: N/A
Is XNNPACK available: True

CPU:
Intel(R) Core(TM) i9-9880H CPU @ 2.30GHz

Versions of relevant libraries:
[pip3] mypy==1.0.0
[pip3] mypy-extensions==1.0.0
[pip3] numpy==1.24.2
[pip3] torch==1.13.1
[pip3] torchvision==0.15.0a0+378a327
[conda] mkl                       2022.2.1         h44ed08c_16952    conda-forge
[conda] numpy                     1.24.2           py39h6ee2318_0    conda-forge
[conda] pytorch                   1.13.1          cpu_py39he8d27a1_1    conda-forge
[conda] torchvision               0.15.0a0+378a327           dev_0    <develop>
@mpearce25
Copy link
Contributor Author

Regarding

torchvision/prototype/datapoints/_datapoint.py:8: error: Module "torch._C" has no attribute "DisableTorchFunctionSubclass"  [attr-defined]
    from torch._C import DisableTorchFunctionSubclass

it seems I may have the same issue discussed in #7126. However, I directly followed the instructions in https://github.com/pytorch/vision/blob/main/CONTRIBUTING.md#install-pytorch-nightly. Using conda during setup I ran conda install pytorch -c pytorch-nightly.

If the contributing guide needs any updates I am also happy to make those as needed.

@pmeier
Copy link
Collaborator

pmeier commented Feb 9, 2023

Hey @mpearce25 and thanks for your interest in contributing. The issues here are twofold:

You are likely using a different version of flake8 that we do:

rev: 5.0.4

Updating that 6.0.0, which is the most recent version, surfaces the errors. I couldn't find release notes, but it seems that flake8 is now able to handle partially unused imports. Meaning from foo import bar, baz was not flagged before if only bar was used, but it is now.

Using conda during setup I ran conda install pytorch -c pytorch-nightly.

Something seem to have gone wrong there, because you have

[conda] pytorch                   1.13.1          cpu_py39he8d27a1_1    conda-forge

in your env. This is the latest stable release, but development happens on the latest nightly release. If you fix that, your mypy errors should also go away. At least I cannot reproduce them.

@mpearce25
Copy link
Contributor Author

Resolved! Thanks for the help @pmeier

The stable version of pytorch was getting installed over the nightly as my .condarc had a channel preference set that super-seeded the -c pytorch-nightly in the install command.

Regarding flake8:

  1. Would it be reasonable to change
    pip install flake8 typing mypy pytest pytest-mock scipy to pip install flake8==5.0.4 typing mypy pytest pytest-mock scipy
    in https://github.com/pytorch/vision/blob/main/CONTRIBUTING.md#install-torchvision ? Or does that introduce too much confusion if the pre-commit hook changes?
  2. I'll open an issue on the Pytorch repo to add flake8 version reporting to https://github.com/pytorch/pytorch/blob/master/torch/utils/collect_env.py. Since flake8 is also a requirement there hopefully it can speed up troubleshooting issues like this across both projects.

@pmeier
Copy link
Collaborator

pmeier commented Feb 10, 2023

Would it be reasonable to change

In general yes, but I would put that into a different section.

https://github.com/pytorch/vision/blob/main/CONTRIBUTING.md#code-formatting-and-typing

TBH, I don't know why flake8 is listed together with the runtime dependencies given that it is only for development. Feel free to send a PR to move that to our lint section and in there pin to the right version like it is done for ufmt, black, and usort.

While you are at it, you can also send a PR to upgrade to flake8==6.0.0.

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

No branches or pull requests

2 participants