Fix pre-commit script to validate files in git tree.

[Godson-Gnanaraj]

Closes #7512

Describe your change:

Python 3.9
Validate only files included in repo by running git ls-files and parsing the output.

• Add an algorithm?
• Fix a bug or typo in an existing algorithm?
• Documentation change?

Checklist:

• I have read CONTRIBUTING.md.
• This pull request is all my own work -- I have not plagiarized.
• I know that pull requests will not be merged if they fail the automated tests.
• This PR only changes one algorithm file. To ease review, please open separate PRs for separate algorithms.
• All new Python files are placed inside an existing directory.
• All filenames are in all lowercase characters with no spaces or dashes.
• All functions and variable names follow Python naming conventions.
• All function parameters and return values are annotated with Python type hints.
• All functions have doctests that pass the automated testing.
• All new algorithms have a URL in its comments that points to Wikipedia or other similar explanation.
• If this pull request resolves one or more open issues then the commit message contains Fixes: #{$ISSUE_NO}.

[cclauss]

#7512 is about scripts/validate_filenames.py.

scripts/build_directory_md.py != scripts/validate_filenames.py

build_dir is run by GitHub Actions, not pre-commit.

Can we upgrade to pathlib instead of using os.path?

[Godson-Gnanaraj]

#7512 is about scripts/validate_filenames.py.
scripts/build_directory_md.py != scripts/validate_filenames.py
build_dir is run by GitHub Actions, not pre-commit.

scripts/validate_filenames.py imports good_file_paths from scripts/build_directory_md.py which returns all the files under top directory.
Fixing good_file_paths fixes #7512.

Can we upgrade to pathlib instead of using os.path?

Sure. I'll upgrade the pull request.

[cclauss]

Oh, Now I understand. scripts/validate_filenames.py imports good_file_paths from scripts/build_directory_md.py.

[tianyizheng02]

@cclauss Is this PR fine to merge?

[cclauss]

This repo runs code on the current Python (3.11 this month, 3.12 next month) so this import is not required.

[tianyizheng02]

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

scripts/build_directory_md.py:11:16: PLW1510 `subprocess.run` without explicit `check` argument
   |
 9 | def get_git_files(top_dir: str = ".") -> Iterable[str]:
10 |     cmd = ["git", "ls-files", top_dir]
11 |     ls_files = subprocess.run(cmd, capture_output=True)
   |                ^^^^^^^^^^^^^^ PLW1510
12 |     paths = iter(ls_files.stdout.decode().strip().split("\n"))
13 |     return paths
   |

scripts/build_directory_md.py:11:31: S603 `subprocess` call: check for execution of untrusted input
   |
 9 | def get_git_files(top_dir: str = ".") -> Iterable[str]:
10 |     cmd = ["git", "ls-files", top_dir]
11 |     ls_files = subprocess.run(cmd, capture_output=True)
   |                               ^^^ S603
12 |     paths = iter(ls_files.stdout.decode().strip().split("\n"))
13 |     return paths
   |

Found 2 errors.

@cclauss For the first error, should check be set to True or False here?

For the second error, I think we can just silence S603 here since the command is hard-coded.

[cclauss]

In both cases we would want check=True because if the command fails then we do not want to proceed.

I question whether we want this PR. It closes an issue that is already closed. Are our current directory inaccurate?

The issue was based on running on Python 3.9 which we never do in this repo. #7592 (comment)

I would rather see us upgrade to https://docs.python.org/3.12/library/pathlib.html#pathlib.Path.walk when Python 3.12 ships next month but I do not see a benefit of calling out to git unless our current directories are inaccurate.

[tianyizheng02]

After looking a bit into the issue myself, I can't reproduce the issue's behavior locally with the current codebase (I'm on Python 3.11.5). Beyond that, I have no reason to believe that our current directories are incorrect, and I don't recall ever encountering the venv issue, for instance, when I work on the repo locally. So I'll just close this issue, and we can open a new one if someone proves us wrong.