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

test_shapefile.py: Fix or suppress some pylint categories. #302

Closed
wants to merge 1 commit into from

Conversation

schwehr
Copy link
Contributor

@schwehr schwehr commented Sep 18, 2024

  • pointless-statement
  • protected-access
  • unused-variable

- pointless-statement
- protected-access
- unused-variable
@JamesParrott
Copy link
Collaborator

Thanks for this. Ruff and other tools are being run on the Python 3 branch. This would be a good fit there. Is it possible to share the Pylint config, and adjust the settings in it?

This PR fixes all the warnings from running Pylint on test_shapefile.py. But there are 68 other warnings from running Pylint on shapefile.py. I don't mind the single "pointless statement" comment, but I'd prefer to avoid cluttering shapefile.py with comments for every tool.

Wouldn't it be better to use for __ in iterable: pass, or more_itertools.consume, instead of using del to call __del__ each iteration? And just not create the new variable shape in the latter cases?

On the Python 3 branch, lets just add an internal dunder property that allows Reader._offsets to be read by the legacy tests that use that implementation detail, without changing the public API.

@JamesParrott
Copy link
Collaborator

JamesParrott commented Sep 19, 2024

I've added a Pylint run (except R,C) on test_shapefile.py to the CI, silenced W0212 using a plug-in for per file settings, and made a few simple changes to the code to avoid these warnings.

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

Successfully merging this pull request may close these issues.

2 participants