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

raster: Fixed PEP8 - E721 for category.py #4758

Merged
merged 2 commits into from
Nov 27, 2024
Merged

Conversation

arohanajit
Copy link
Contributor

@arohanajit arohanajit commented Nov 25, 2024

Used isinstance() instead of == for type instance checks

Arohan Ajit added 2 commits November 25, 2024 12:51
Copy link
Member

@echoix echoix left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You might want to look if these issues are also ignored by ruff with the following rule : https://docs.astral.sh/ruff/rules/type-comparison/

Using ruff might help you find these faster too, and then use flake8 for the exhaustive checking of the unhandled cases from ruff.

@github-actions github-actions bot added Python Related code is in Python libraries labels Nov 25, 2024
@echoix
Copy link
Member

echoix commented Nov 25, 2024

I see in the pyproject.tomm that E721 is completely ignored for now.

grass/pyproject.toml

Lines 127 to 134 in 63fc743

"DTZ011", # call-date-today
"E402", # module-import-not-at-top-of-file
"E501", # line-too-long
"E721", # type-comparison
"E722", # bare-except
"E731", # lambda-assignment
"E741", # ambiguous-variable-name
"F403", # undefined-local-with-import-star

@echoix
Copy link
Member

echoix commented Nov 25, 2024

See here: echoix#305, there's only 23 instances of E721 detected by ruff, and you're already fixing 2 of them. Are some of them easily fixable too?

For this particular rule, they are usually quite easy to review, once the choice on whether using an isinstance check is appropriate vs an is / is not check.

@echoix
Copy link
Member

echoix commented Nov 27, 2024

@arohanajit Is there something else to do here, after verifications? Otherwise we could go forward with this one

@arohanajit
Copy link
Contributor Author

@arohanajit Is there something else to do here, after verifications? Otherwise we could go forward with this one

No, for the flake8 errors in this particular case there is nothing more to be done

@echoix echoix merged commit 115a3c4 into OSGeo:main Nov 27, 2024
27 checks passed
@github-actions github-actions bot added this to the 8.5.0 milestone Nov 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
libraries Python Related code is in Python
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants