pylint-dev / pylint

It's not just a linter that annoys you!
https://pylint.readthedocs.io/en/latest/
GNU General Public License v2.0
5.26k stars 1.12k forks source link

False positive unnecessary-negation for float comparison #9741

Open NicoB77 opened 3 months ago

NicoB77 commented 3 months ago

Bug description

The code below causes unnecessary-negation, which is wrong. For float comparisons, "not a < b" and "b <= a" are not equivalent: if a or b is nan, the first condition is true while the second is false.

target = float(target)
if not 0.0 < target:
    raise ValueError('Target must be positive!')

Command used

pylint --disable=RP0001 --disable=RP0003 --disable=RP0101 --disable=RP0401 --disable=RP0701 --disable=RP0801

Pylint output

************* Module a
a.py(134): [C0117 A.B] Consider changing "not 0.0 < target" to "0.0 >= target" (unnecessary-negation)

Expected behavior

No warning

Pylint version

pylint 3.2.3
astroid 3.2.2
Python 3.10.6 (tags/v3.10.6:9c7b4bd, Aug  1 2022, 21:53:49) [MSC v.1932 64 bit (AMD64)]
Pierre-Sassoulas commented 3 months ago

Thank you for opening the issue.

I feel like the nan should be handled in another conditional because explicit is better than implicit:

target = float(target)
if math.isnan(target) or target <= 0.0:
    raise ValueError('Target must be positive!')

Not sure if we should upgrade the check's message to add math.isnan to all the suggestions though. Might be a little verbose and not relevant most of the time. Or it might be a nice "beware of nan" reminder that actually helps user ?

NicoB77 commented 3 months ago

Thanks for your reply. The explicit call of isnan looks too verbose to me, I would rather not add this to the suggestions. Since the usual equivalences of comparisons do not hold for floats, I am not sure if there should be a warning in the first place. As a workaround I have disabled this warning for my project.