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

Return NotImplemented sooner for ErrorDetail equality test #7531

Merged
merged 1 commit into from
Oct 9, 2020

Conversation

smithdc1
Copy link
Contributor

The test suite raises warnings when tested against Python 3.9.
DeprecationWarning: NotImplemented should not be used in a boolean context

In __eq__ for ErrorDetail is where the warning is being raised. This is due to r potentially returning NotImplemented. This PR captures when this takes place and returns NotImplemented sooner.

The test suite raises warnings when tested against Python 3.9
`DeprecationWarning: NotImplemented should not be used in a boolean context`

Where `r` returns `NotImplemented` then this change returns `NotImplemented` first to avoid the comparison test.
Copy link
Contributor

@adamchainz adamchainz left a comment

Choose a reason for hiding this comment

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

Makes sense. You could add a regression test to check the warning is not raised with recwarn - https://docs.pytest.org/en/latest/warnings.html#ensuring-code-triggers-a-deprecation-warning

@adamchainz
Copy link
Contributor

No need for a regression test if we fail on deprecation warnings from #7586 .

@adamchainz adamchainz merged commit 79c37d0 into encode:master Oct 9, 2020
@smithdc1 smithdc1 deleted the ErrorDetail_comparison branch October 9, 2020 11:36
tomchristie pushed a commit that referenced this pull request Aug 1, 2022
…#8538)

PR #7531 resolved issue #7433 by updating `ErrorDetails.__eq__` to correctly
handle the `NotImplemented` case. However, Python 3.9 continues to issue the
following warning:

    DeprecationWarning: NotImplemented should not be used in a boolean context

This is because `__ne__` still doesn't handle the `NotImplemented` case
correctly. In order to avoid this warning, this commit makes the same change
for `__ne__` as previously made for `__eq__`.
sigvef pushed a commit to sigvef/django-rest-framework that referenced this pull request Dec 3, 2022
The test suite raises warnings when tested against Python 3.9
`DeprecationWarning: NotImplemented should not be used in a boolean context`

Where `r` returns `NotImplemented` then this change returns `NotImplemented` first to avoid the comparison test.
sigvef pushed a commit to sigvef/django-rest-framework that referenced this pull request Dec 3, 2022
…encode#8538)

PR encode#7531 resolved issue encode#7433 by updating `ErrorDetails.__eq__` to correctly
handle the `NotImplemented` case. However, Python 3.9 continues to issue the
following warning:

    DeprecationWarning: NotImplemented should not be used in a boolean context

This is because `__ne__` still doesn't handle the `NotImplemented` case
correctly. In order to avoid this warning, this commit makes the same change
for `__ne__` as previously made for `__eq__`.
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.

4 participants