The git pre-commit hook contradices itself and goes beyond the scope of the files to commit

[carlos3dx]

Repository commit

e6d825e

Python version (python --version)

Python 3.9.1

Dependencies version (pip freeze)

absl-py==1.3.0
astunparse==1.6.3
beautifulsoup4==4.11.1
cachetools==5.2.0
certifi==2022.9.24
cffi==1.15.1
charset-normalizer==2.1.1
contourpy==1.0.5
cryptography==38.0.1
cycler==0.11.0
dill==0.3.5.1
fake-useragent==0.1.11
flatbuffers==22.9.24
fonttools==4.37.4
gast==0.4.0
google-auth==2.13.0
google-auth-oauthlib==0.4.6
google-pasta==0.2.0
grpcio==1.49.1
h5py==3.7.0
idna==3.4
importlib-metadata==5.0.0
joblib==1.2.0
keras==2.10.0
Keras-Preprocessing==1.1.2
kiwisolver==1.4.4
libclang==14.0.6
lxml==4.9.1
Markdown==3.4.1
MarkupSafe==2.1.1
matplotlib==3.6.1
mpmath==1.2.1
networkx==2.8.7
ntlm-auth==1.5.0
numpy==1.23.4
oauthlib==3.2.1
opencv-python==4.6.0.66
opt-einsum==3.3.0
packaging==21.3
pandas==1.5.0
patsy==0.5.3
pbr==5.10.0
Pillow==9.2.0
pip==21.3.1
ply==3.11
projectq==0.8.0
protobuf==3.19.6
psutil==5.9.2
pyasn1==0.4.8
pyasn1-modules==0.2.8
pycparser==2.21
pyparsing==3.0.9
python-dateutil==2.8.2
pytz==2022.4
qiskit==0.39.0
qiskit-aer==0.11.0
qiskit-ibmq-provider==0.19.2
qiskit-terra==0.22.0
requests==2.28.1
requests-ntlm==1.1.0
requests-oauthlib==1.3.1
retworkx==0.12.0
rsa==4.9
rustworkx==0.12.0
scikit-fuzzy==0.4.2
scikit-learn==1.1.2
scipy==1.9.2
setuptools==60.2.0
six==1.16.0
sklearn==0.0
soupsieve==2.3.2.post1
statsmodels==0.13.2
stevedore==4.0.1
symengine==0.9.2
sympy==1.11.1
tensorboard==2.10.1
tensorboard-data-server==0.6.1
tensorboard-plugin-wit==1.8.1
tensorflow==2.10.0
tensorflow-estimator==2.10.0
tensorflow-io-gcs-filesystem==0.27.0
termcolor==2.0.1
texttable==1.6.4
threadpoolctl==3.1.0
tk==0.1.0
tweedledum==1.1.1
tweepy==4.10.1
typing_extensions==4.4.0
urllib3==1.26.12
websocket-client==1.4.1
websockets==10.3
Werkzeug==2.2.2
wheel==0.37.1
wrapt==1.14.1
xgboost==1.6.2
yulewalker==0.1.1
zipp==3.9.0

Expected behavior

When executing the pre-commit hook it only should only focus on the files that are being committed and ignore the ones in fit ignored folder like env.

Type hinting with typing library should be respected.

Actual behavior

When executing the pre-commit hook the file formatter complains about files that are not going to be committed and are in an ignored folder, for example, env.

Also if a function has type hinting from typing, it converts foo:List[str] to foo:list[str], and later complains that it is not subscriptable and also for the the unused import from typing import List cause the type hinting is not longer using the imported one. But if you don't do type hinting, the suggested hint includes the ones from typing

[cclauss]

I see two separate issues here:

1. validate-filenames is looking at files in venv and it should not be.
2. pyupgrade is going to force foo: List[str] --> foo: list[str] but it does not remove the import so flake8 which runs right after it and should be raising an F401 but it is not?

later complains that it is not subscriptable

Can you please provide the error text?

[Godson-Gnanaraj]

2. pyupgrade is going to force `foo: List[str]` --> `foo: list[str]` but it does not remove the import so flake8 which runs right after it and should be raising an [`F401`](https://www.flake8rules.com/rules/F401.html) but it is not?

from __future__ import annotations

Annotations become strings at runtime with above import. So type hints will not be affected by pyupgrade.

[carlos3dx]

I see two separate issues here:

1. validate-filenames is looking at files in venv and it should not be.
2. pyupgrade is going to force foo: List[str] --> foo: list[str] but it does not remove the import so flake8 which runs right after it and should be raising an F401 but it is not?

later complains that it is not subscriptable

Can you please provide the error text?

Examples from the open PR I made: #7509

1: That's right, example:

Validate filenames.......................................................Failed
- hook id: validate-filenames
- exit code: 145

374 files contain uppercase characters:
venv/lib/python3.9/site-packages/sympy/liealgebras/tests/test_type_F.py
venv/lib/python3.9/site-packages/sympy/liealgebras/tests/test_type_B.py
venv/lib/python3.9/site-packages/sympy/liealgebras/tests/test_type_C.py
venv/lib/python3.9/site-packages/sympy/liealgebras/tests/test_type_G.py
venv/lib/python3.9/site-packages/sympy/liealgebras/tests/test_type_D.py
venv/lib/python3.9/site-packages/sympy/liealgebras/tests/test_type_E.py
venv/lib/python3.9/site-packages/sympy/liealgebras/tests/test_type_A.py
venv/lib/python3.9/site-packages/fontTools/ttx.py
venv/lib/python3.9/site-packages/fontTools/fontBuilder.py
venv/lib/python3.9/site-packages/fontTools/unicode.py
venv/lib/python3.9/site-packages/fontTools/tfmLib.py
venv/lib/python3.9/site-packages/fontTools/afmLib.py
venv/lib/python3.9/site-packages/fontTools/agl.py
venv/lib/python3.9/site-packages/fontTools/help.py
venv/lib/python3.9/site-packages/fontTools/__main__.py
[...]
venv/lib/python3.9/site-packages/lxml/html/ElementSoup.py
venv/lib/python3.9/site-packages/scipy/optimize/moduleTNC.py

10267 files contain hyphen characters:
venv/lib/python3.9/site-packages/texttable.py
venv/lib/python3.9/site-packages/threadpoolctl.py
venv/lib/python3.9/site-packages/pylab.py
venv/lib/python3.9/site-packages/zipp.py
venv/lib/python3.9/site-packages/_virtualenv.py
[...]
venv/share/doc/networkx-2.8.7/examples/basic/plot_read_write.py
venv/share/doc/networkx-2.8.7/examples/basic/plot_properties.py
venv/share/doc/networkx-2.8.7/examples/basic/plot_simple_graph.py

Every file flagged is from venv.

2: First the signature was like this:

from typing import Any, Dict, List



def viterbi(
    observations_space: List[str],
    states_space: List[str],
    initial_probabilities: Dict[str, float],
    transition_probabilities: Dict[str, Dict[str, float]],
    emission_probabilities: Dict[str, Dict[str, float]],
) -> List[str]:

After trying to commit this message is shown:

$ git commit --file=/var/folders/rw/fw7dpn3558s698gj8mx1042w0000gn/T/50CFA53A-ED81-4B23-8653-DBA87565C72C

check that executables have shebangs.................(no files to check)Skipped
check yaml...........................................(no files to check)Skipped
fix end of files.........................................................Passed
trim trailing whitespace.................................................Passed
fix requirements.txt.................................(no files to check)Skipped
black....................................................................Passed
isort....................................................................Passed
pyupgrade................................................................Failed
- hook id: pyupgrade
- exit code: 1
- files were modified by this hook

Rewriting dynamic_programming/viterbi.py

flake8...................................................................Failed
- hook id: flake8
- exit code: 1

dynamic_programming/viterbi.py:1:1: F401 'typing.Dict' imported but unused
dynamic_programming/viterbi.py:1:1: F401 'typing.List' imported but unused

mypy.....................................................................Failed
- hook id: mypy
- exit code: 1

dynamic_programming/viterbi.py:5: error: "list" is not subscriptable, use "typing.List" instead
dynamic_programming/viterbi.py:6: error: "list" is not subscriptable, use "typing.List" instead
dynamic_programming/viterbi.py:7: error: "dict" is not subscriptable, use "typing.Dict" instead
dynamic_programming/viterbi.py:8: error: "dict" is not subscriptable, use "typing.Dict" instead
dynamic_programming/viterbi.py:9: error: "dict" is not subscriptable, use "typing.Dict" instead
dynamic_programming/viterbi.py:10: error: "list" is not subscriptable, use "typing.List" instead
Found 6 errors in 1 file (checked 1 source file)

codespell................................................................Passed

pyupgrade changed the hints from the typing library to the data types (Dict to dict)

def viterbi(
    observations_space: list[str],
    states_space: list[str],
    initial_probabilities: dict[str, float],
    transition_probabilities: dict[str, dict[str, float]],
    emission_probabilities: dict[str, dict[str, float]],
) -> list[str]:

flake8 complains about Dict and List not being used, cause their usage was removed.

And finally mypy complains cause the type hinting is no longer correct

[carlos3dx]

What version of Python are you using?

python --version
Python 3.9.1

[cclauss]

Adding from __future__ import annotations should fix this.

A fascinating tidbit in the Python 3.11 docs.

• https://docs.python.org/3.11/whatsnew/3.11.html#pep-563-may-not-be-the-future

[Godson-Gnanaraj]

@carlos3dx could you please confirm #7592 fixes validate_filename check?

[cclauss]

Yes. @carlos3dx it does but I think we should move that process back out of pre-commit and into GitHub Actions. Our regular contributors (those who would actually have pre-commit installed and working) do not make filename errors.

The folks who generate filename errors are our first-time contributors (we have a much higher proportion of these folks than other repos). This cohort almost never has pre-commit installed and working. Given that this script does not currently fix the errors (that would be way too confusing!) we can move it back over to GHA.

[carlos3dx]

@carlos3dx could you please confirm #7592 fixes validate_filename check?

Yes, it fixes the issue with related to checkin the files on venv.

[Godson-Gnanaraj]

@cclauss Does that mean we need to remove validate_filenames from pre-commit hook?

[cclauss]

Let's add it to GitHub Actions first. I have 24 hours left in Hactoberfest and 200+ open PR is this is not my highest priority.