Fix unused global warnings from new flake8 7.2.0#1609
Conversation
parthosa
added
bug
parthosa
requested review from
amahussein,
cindyyuanjiang,
Copilot and
sayedbilalbari
There was a problem hiding this comment.
Pull Request Overview
This PR fixes warnings raised by flake8 7.2.0 by removing unused nonlocal declarations and updates the copyright year.
- Updated copyright year from 2024 to 2025.
- Removed unused nonlocal variables (dir_patterns and file_patterns) from the inner function.
Comments suppressed due to low confidence (1)
user_tools/src/spark_rapids_pytools/common/sys_storage.py:242
- Confirm that the removal of 'dir_patterns' and 'file_patterns' from the nonlocal declaration does not affect any remaining references in the function. If these variables are no longer used, consider removing their declarations from the outer scope as well.
nonlocal files_count, dir_count
Signed-off-by: Partho Sarthi <[email protected]>
amahussein
left a comment
amahussein
left a comment
There was a problem hiding this comment.
Thanks @parthosa
QQ if this works on Py [3.9, 3.12]?
|
|
||
| def inner(dir_p: pathlib.Path, prefix: str = '', level=-1): | ||
| nonlocal files_count, dir_count, dir_patterns, file_patterns | ||
| nonlocal files_count, dir_count |
There was a problem hiding this comment.
I see dir_patterns and file_patterns are being used.
I guess it is because it is nort needed to be redefined as non_local.
Would this work for python 3.9+?
There was a problem hiding this comment.
That is correct. Since these are read-only variables (defined in the outer function), we do not need them to be non_local.
Yes, all tox tests passed for python versions 3.9-3.12.
There was a problem hiding this comment.
AFAIK, Tox is not a 100% reliable way to tell because if the unit-tests do not cover that path, we won't know that something is wrong in runtime.
There was a problem hiding this comment.
Yes. That is true. In this case, I think it should be fine because these variables are read-only and these variables can be safely read in the inner function.
amahussein
left a comment
amahussein
left a comment
There was a problem hiding this comment.
It is interesting to see that co-pilot reviews are enabled on our repo.
Thanks @parthosa !
P.s: check the comment about Tox.
LGTME
Thanks @amahussein. I agree tox does not guarantee the coverage. But in this case, I think it should be fine because these variables are read-only and hence can be safely read in the inner function. |
Fixes #1608.
This PR fixes the code for a new warning added in the newly released flake8: PyCQA/pyflakes#825.