Is using Python `isinstance` ever right?

Question

I've got a 2D array of different blocks, all inheriting from Block. I want to check if the block that I clicked on is a Dirt type block, like this:

clickedblock = getClickedBlock()

if isinstance(clickedblock, Dirt):
    place a block

else:
    don't place a block

I've heard that isinstance is bad, and should be avoided because it creates forks in code. What times would isinstance be good to use?

Another more cumbersome solution for my problem would be to have a field of Block called 'id' and then check if it equals some constant that means Dirt. But that sounds quite bad and more prone for mistake than the simple isinstance.

@Keyser: That's exactly my point. You disagree, but you don't tell me why you disagree, so you don't give other people the chance to make up their own minds if they so choose. — Dietrich Epp, Nov 29 '12 at 22:51
@DietrichEpp Me not stating my reasons doesn't prevent people from making up their own minds. Perhaps even the opposite. The reason I didn't was because it's not the right forum :) — keyser, Nov 29 '12 at 22:52
I suppose the most object oriented way would be something like `if clickedblock.is_placeable(position): ...` or `try: clickedblock.place(position)` — Steven Rumbalski, Nov 29 '12 at 22:53
@DietrichEpp the beginning of your statement is a bit condescending. For all you know, they could be a child (in fact it looks like OP was around 14 when the question was asked). It also implies that asking such a question is childish (it's not). BTW, whether someone tells you if something is good or bad is besides the point. Programmers are interested in knowing when `isinstance` is justified and when it should be avoided. — Dennis, Nov 05 '14 at 00:31
@Dennis: My complaint is about saying "X is bad"... I think that "X is bad" is a condescending phrase, omits useful information, and such statements should be taken with a grain of salt. It's the kind of thing you say to a child when you don't want to give any justifications for your advice. — Dietrich Epp, Nov 05 '14 at 01:26

score 14 · Accepted Answer · answered Nov 29 '12 at 22:55

Your example seems like a legitimate use case of isinstance().

It's not that isinstance() is bad, often polymorphism can be used for the same purpose (which results in cleaner code in where the class is used).

But sometimes, isinstance() is what you need. For example, the pythonic way of detecting whether a variable is string or not is isinstance(var, basestring).

bawey · Answer 2 · 2019-03-28T09:04:18.807

3

I learned the hard way against using it. The problem is that the outcome is sensitive to how the class definitions were imported:

in the place where the object was instantiated
in the place where the isinstance test is performed

If one import was relative and the other absolute - the check will fail. Essentially it'll be like checking on equality between SomeClass vs somepackage.SomeClass. It doesn't even matter that they come from the same file and so on. Also, there will be a similar outcome if you somehow have both the root directory and somepackage directory in your PYTHONPATH - then an absolute-styled import could denote a path from either of the "source roots" so two different absolute-styled imports would result in failing instance checks.

One could argue about how good practices would anyway prevent that from happening but good practices are also largely about not taking chances. In that spirit I prefer to put some abstract method in a common ancestor class so that I can later rely on how things quack rather than on what the interpreter believes them to be.

In Java each class resolves to its fully qualified class name. These are unique within a program and tests for instances are a breeze. In Python these can be slippery.

edited Mar 28 '19 at 09:04

answered Mar 28 '19 at 08:54

bawey

149
1
4

1

The problem you are describing occurs when the same module is imported twice under different names and Python cannot tell that those two names reference the same file. This is a bug in your code, not a problem with `isinstance`. This is also exactly why you don't put the internals of your package into `PYTHONPATH` (or `sys.path`). – Ethan Furman Dec 17 '20 at 23:56
Geez, I've already written it's all about imports. The "you have a bug" point becomes less valid if you work with a library or someone else's code and there is no time to drill down to the bottom of that sort of behaviour. – bawey Dec 18 '20 at 07:12
You might as well say "you've learned the hard way not to use `if`" because some library messed up the `__bool__` method on some of their objects. Bugs in people's code is not a good reason to categorically stop using a core language feature, which `isinstance` absolutely is. – Ethan Furman Dec 18 '20 at 22:10
Well, in all honesty I agree with you: it's often better to check explicitly if something is empty, is a `None` or, ironically, an instance of `LocalProxy` that ain't `None` until you start working on it (looking at you Flask and Werkzeug) rather than relying on mere `if(my_var)`. My bias might be unfair, my wording poor but `isinstance` can be a hell of a surprise to someone coming from a different language background. If that makes me a mediocre dev - so be it - certain limits I'll never surpass. Meanwhile I simply prefer tools that don't trip me up. Cheers. – bawey Dec 19 '20 at 11:10

pjz · Answer 3 · 2012-11-30T04:52:15.817

1

I think I'd change it to be more like:

PLACEABLE_TYPES = [ Dirt ]
if isinstance(clickedblock, PLACEABLE_TYPES):
   place the block
else:
   don't place the block

but the idea in the comments of:

if clickedblock.is_placeable(that_place):
    place the block
else:
    don't place the block

also has merit.

edited Nov 30 '12 at 04:52

answered Nov 29 '12 at 23:00

pjz

41,842
6
48
60

1

The first example is almost certainly incorrect! It will fail if the `clickedblock` is a subclass of `Dirt`. Better would be: `if isinstance(clickedblock, PLACEABLE_TYPES)`. But the second example, `clickedblock.is_placeable(…)`, is likely much better. – David Wolever Nov 29 '12 at 23:34

score 1 · Answer 4 · edited Dec 17 '20 at 23:57

1

If you don't want to use it, you've got other options. Traditional duck typing solution:

try:
    clickedblock_place = clickedblock.place
except AttributeError:
    # don't place block
else:
    clickedblock_place()

Or you can use hasattr:

if hasattr(clickedblock, 'place'):
    clickedblock.place()

I hardly ever use isinstance except for checking up (or is it down?) the inheritance hierarchy, say, for instance, if you need to know if a name points to a str OR a unicode:

if isinstance(str1, basestring):
    blah, blah, blah

edited Dec 17 '20 at 23:57

Ethan Furman

63,992
20
159
237

answered Nov 29 '12 at 23:31

MikeHunter

4,144
1
19
14

4

*Please don't use `except:`*! It's terrible, horrible, and it will probably give your children some incurable desease. Using `except:` will catch *all* exceptions, which will make debugging legitimate bugs in `.place()` a nightmare, catch ctrl-c, and a host of other undesirable things. – David Wolever Nov 29 '12 at 23:36
1

If you want to use `try/except` (not *necessarily* bad), use: `try: place_block = clickedblock.place; except AttributeError: dont_place(); else: place_block()` - that way you will see legitimate bugs in the `.place()` method. – David Wolever Nov 29 '12 at 23:38
1

Thanks for the edit @David Wolever. You are of course exactly right. I was in a hurry. – MikeHunter Nov 30 '12 at 00:24

Is using Python `isinstance` ever right?

4 Answers4