Skip to content

Conversation

@flauschzelle
Copy link
Contributor

While I was setting up the dev environment, I ran the tests as described in the readme, and got some error messages from flake8:

tests/validators/allow_empty_string_test.py:33:16: E721 do not compare types, for exact checks use `is` / `is not`, for instance checks use `isinstance()`
tests/validators/allow_empty_string_test.py:49:16: E721 do not compare types, for exact checks use `is` / `is not`, for instance checks use `isinstance()`
tests/validators/none_to_unset_value_test.py:34:16: E721 do not compare types, for exact checks use `is` / `is not`, for instance checks use `isinstance()`
tests/validators/noneable_test.py:34:16: E721 do not compare types, for exact checks use `is` / `is not`, for instance checks use `isinstance()`
tests/validators/noneable_test.py:50:16: E721 do not compare types, for exact checks use `is` / `is not`, for instance checks use `isinstance()`

I fixed the style according to these messages, now the tests run without flake8 complaining again :)

Copy link
Contributor

@binaryDiv binaryDiv left a comment

Choose a reason for hiding this comment

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

Usually isinstance would be the way to go. However, since these are all assertions in the unit tests to check the type of the validated result, I think a strict test with type(result) is type(expected_result) would be better.

If the validator results with an object of a subclass of the expected type, something is wrong.

@flauschzelle flauschzelle requested a review from binaryDiv April 9, 2024 14:39
@binaryDiv binaryDiv merged commit e9d5551 into main Apr 9, 2024
@binaryDiv binaryDiv deleted the fix-type-comparison-style branch April 9, 2024 14:41
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.

3 participants