Multiple instances of Python object-testing mistakes

Bug #2000000 reported by FeRD (Frank Dana)
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
HPLIP
New
Undecided
Unassigned

Bug Description

The HPLIP Python code contains multiple instances where a string literal is compared with a variable using identity comparison ("is" / "is not"), which as the language reference will tell you (and a warning output to the terminal notes) is generally ill-advised.

IOW, taking an example from ui5/devmgr5.py:

if self.latest_available_version is not "":

The way to write such a comparison without triggering a SyntaxWarning is,

if self.latest_available_version != "":

"is not" and "is" should generally only be used to check whether two variables refer to the _exact same object_ or literal. The most common uses are "somevar is None" or "somevar is not None". None is a singleton object, so there can only ever be one instance of None in the runtime.

Additionally, base/pexpect/__init__.py contains an order-of-evaluation error on line 789:

if timeout < 0 and timeout is not None:

if timeout ever actually is None, attempting to compare it to an integer will trigger an exception, "TypeError: '<' not supported between instances of 'NoneType' and 'int'". To properly short-circuit the evaluation, the order of the two tests must be reversed.

The attached patch contains fixes for the pexpect condition, and replaces all instances of "is not <string literal>" that I could find with "!= <string literal>". I can't guarantee there aren't still others lurking in the code somewhere, though.

Revision history for this message
FeRD (Frank Dana) (ferdnyc) wrote :
Revision history for this message
FeRD (Frank Dana) (ferdnyc) wrote :

(Wow! 2 millionth bug. I feel like there should be balloons or something.)

Revision history for this message
Colin Watson (cjwatson) wrote :

Launchpad staff here (though on holiday). Here are some balloons and a party popper for you: πŸŽˆπŸŽˆπŸŽˆπŸŽ‰

We now return you to your regularly scheduled actually useful bug conversation, I hope.

Revision history for this message
FeRD (Frank Dana) (ferdnyc) wrote :

Heh! Thanks, Colin.

Actually not THAT useful. I meant to follow up on this to say: "Nope, I'm mostly wrong."

* It's true that identity comparisons between variables and literals are ill-advised.
* It's true that they cause a SyntaxWarning to be emitted. (...WTF is a "syntax WARNING"?!?)
* However, they still WORK in Python, because... *sigh* because they just do.

So, while it's (IMHO) worth fixing these, it's not nearly as urgent/imperative as I made it sound.

(Except for the pexpect order-of-operations one, that's genuinely a bug.)

summary: - Multiple instances of Python object-testing errors
+ Multiple instances of Python object-testing mistakes
description: updated
Revision history for this message
Eric Chen (eric-chen) wrote (last edit ):

Congratulation! 2 millionth bug! πŸŽ‰

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.