Clarify motivation for E713 and E714 (#11483)

The wording 'negative comparison' is a rather vague description of the
'is not' operation and does not describe what the 'not in' operation
does (potentially copied from 'is not'). This was replaced with more
precise language to describe the operators taken from the official
python docs[1].

Both rules didn't have a strong reasoning besides 'it's bad, use the
other'. The origin of these rules seems to be PEP8[2] which prefers 'is
not' over 'not ... is' for readability. This is now reflected in the
description.

[1]:
https://docs.python.org/3/reference/expressions.html#membership-test-operations
[2]: https://peps.python.org/pep-0008/#programming-recommendations
This commit is contained in:
Nicolas Jeker 2024-05-21 21:12:18 +02:00 committed by GitHub
parent 83b8b62e3e
commit 84531d1644
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
1 changed files with 7 additions and 4 deletions

View File

@ -9,10 +9,10 @@ use crate::checkers::ast::Checker;
use crate::registry::Rule;
/// ## What it does
/// Checks for negative comparison using `not {foo} in {bar}`.
/// Checks for membership tests using `not {element} in {collection}`.
///
/// ## Why is this bad?
/// Negative comparison should be done using `not in`.
/// Testing membership with `{element} not in {collection}` is more readable.
///
/// ## Example
/// ```python
@ -42,10 +42,11 @@ impl AlwaysFixableViolation for NotInTest {
}
/// ## What it does
/// Checks for negative comparison using `not {foo} is {bar}`.
/// Checks for identity comparisons using `not {foo} is {bar}`.
///
/// ## Why is this bad?
/// Negative comparison should be done using `is not`.
/// According to [PEP8], testing for an object's identity with `is not` is more
/// readable.
///
/// ## Example
/// ```python
@ -60,6 +61,8 @@ impl AlwaysFixableViolation for NotInTest {
/// pass
/// Z = X.B is not Y
/// ```
///
/// [PEP8]: https://peps.python.org/pep-0008/#programming-recommendations
#[violation]
pub struct NotIsTest;