pylint warnings #6659
pylint warnings #6659
Conversation
Fixed some pylint issues
for more information, see https://pre-commit.ci
|
Not quite sure what to do about the coverage - it's not like I changed functionality much, per se |
|
I've created PR #6660 to help one of the missing coverage lines. As for the other, it's being skipped on CI jobs, so there's nothing to do there. |
| def test_truncated_file(tmp_path): | ||
| # Test EOF in header | ||
| path = str(tmp_path / "temp.pgm") | ||
| with open(path, "w") as f: | ||
| with open(path, "w", encoding="utf-8") as f: | ||
| f.write("P6") | ||
|
|
||
| with pytest.raises(ValueError) as e: |
There was a problem hiding this comment.
ppm/pgm files are either ASCII or binary. Since we're writing text here we should probably use "ascii".
There was a problem hiding this comment.
Since this is P6, it is binary.
I've created PR #6677 to change this to write in binary format.
|
Just checking - so are we adding another linting standard? I had thought the idea behind using black was to end style debates.
They will not always agree. https://black.readthedocs.io/en/stable/guides/using_black_with_other_tools.html#pylint
|
There was a problem hiding this comment.
Black is about style, generally about whitespace and doesn't touch the AST, whereas Pylint is a static code analyser, and also checks for code deprecations. It can help find bugs.
Pylint has lots of suggestions, and should be evaluated on a case-by-case basis.
For example, we're not going to change:
C0103: Variable name "im" doesn't conform to snake_case naming style (invalid-name)
I think these changes are fine.
Fixed some pylint issues
Fixes # unnecessary elses, conditional checks, import structure and encodings
Changes proposed in this pull request: