classification
Title: suggest assertIs(type(obj), cls) for exact type checking
Type: enhancement Stage: resolved
Components: Documentation, Tests Versions: Python 3.2, Python 3.3, Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: ezio.melotti Nosy List: docs@python, eric.araujo, eric.snow, ezio.melotti, flox, georg.brandl, michael.foord, pitrou, python-dev, rhettinger
Priority: normal Keywords: patch

Created on 2011-11-12 12:43 by flox, last changed 2011-12-19 05:08 by ezio.melotti. This issue is now closed.

Files
File name Uploaded Description Edit
grep_test_is_instance.log flox, 2011-11-12 14:49
grep_test_exact_type.log flox, 2011-11-12 14:50
issue13387_exact_type.diff flox, 2011-11-12 14:52 review
Messages (18)
msg147480 - (view) Author: Florent Xicluna (flox) * (Python committer) Date: 2011-11-12 12:43
Sometimes we want to check the exact type of an object.
The method assertIsInstance could be misleading in such case.

The current workaround is:
assertIs(type(obj), some_class)

However we can add an argument to the method to keep the benefit of  having a nice failure message.

Examples:
  assertIsInstance(stdobj, dict, exact_type=True)
  assertIsInstance(myobj, dict, exact_type=MyDict)
msg147482 - (view) Author: Ezio Melotti (ezio.melotti) * (Python committer) Date: 2011-11-12 13:40
I'm not sure this is common enough to justify a new arg.  The status quo has the advantage that is quite close with what we would use in an 'if' statement (i.e. either "if isinstance(obj, some_class):" or "if type(obj) is some_class:").
msg147494 - (view) Author: Florent Xicluna (flox) * (Python committer) Date: 2011-11-12 14:52
At least in the standard library test suite, it should be useful.
When the test is stricter, it catches errors earlier.

I remember when assertIsInstance was made available (issue #7031), we started to rewrite some expressions
  self.assertEqual(type(result), str)
with
  self.assertIsInstance(result, str)

Actually, it means we relaxed the test.
Today, I don't want to use assertIsInstance anymore because I want to check the exact type() of the result.

The attached files list the usage of both in Lib/test/*py:
 - 325 assertIsInstance
 - 234 assert. . .type(. . .)

IMHO, some assertIsInstance can be stricter.
Most of the other type() tests can be replaced with this method.
msg147495 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2011-11-12 14:55
I find that assertIs(type(x), someclass) is very clear, whereas an assertIsInstance that would not behave like isinstance depending on one argument would be non-obvious to me.
msg147501 - (view) Author: Florent Xicluna (flox) * (Python committer) Date: 2011-11-12 15:25
I would say that this one is clear too:
  aelf.assertTrue(isinstance(obj, cls))

except that the failure message is not very friendly:
  AssertionError: False is not true


If we keep assertIsInstance, more people will continue to misuse it just because the method exist, when they really want to check (type(obj) is cls).

An option could be to add a snippet to the documentation of `assertIsInstance` stating that the right way to check exact type is `assertIs(type(obj), cls)`.
msg147504 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2011-11-12 15:32
>I would say that this one is clear too:
>  aelf.assertTrue(isinstance(obj, cls))
> except that the failure message is not very friendly:
>  AssertionError: False is not true
Yeah, you have to give something as third argument to ease debugging.

> If we keep assertIsInstance, more people will continue to misuse it just because the
> method exist, when they really want to check (type(obj) is cls).
If they make that mistake, it is because they don’t understand what isinstance does.

> An option could be to add a snippet to the documentation of `assertIsInstance` stating
> that the right way to check exact type is `assertIs(type(obj), cls)`.
My point was that maybe they think they really want to check the type, but with Python you don’t have to care that much most of the time.

+1 on a doc addition (I can even volunteer a patch)
-0.5 on the proposed new argument
msg147512 - (view) Author: Michael Foord (michael.foord) * (Python committer) Date: 2011-11-12 17:49
I think your proposed workaround is good enough and no extra effort to type than the suggested change to assertIsInstance.

-1 on a new method

I think the behaviour of isinstance is clear enough that people who misunderstand what assertIsInstance is doing have a problem with basic Python - and will continue to make the mistake whatever we do to assertIsInstance.
msg147513 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2011-11-12 17:53
I don't think there's a point in adding such an extra argument.
Why don't you just write
    self.assertIs(type(myobj), sometype)

? How is the error message not good enough?
msg147514 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2011-11-12 18:05
@msg147513
> Why don't you just write
>     self.assertIs(type(myobj), sometype)

+1
msg147523 - (view) Author: Florent Xicluna (flox) * (Python committer) Date: 2011-11-12 19:12
> +1 on a doc addition (I can even volunteer a patch)

I agree we can highlight the difference between assertIs(type(obj), cls) and assertIsInstance(obj, cls) in the documentation.

Let's forget this patch and keep it simple.
msg147898 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2011-11-18 17:01
New changeset fd9d7a8e45bc by Ezio Melotti in branch '2.7':
#13387: add note about checking the exact type in assertIsInstance doc.
http://hg.python.org/cpython/rev/fd9d7a8e45bc

New changeset 583aff635ce1 by Ezio Melotti in branch '3.2':
#13387: add note about checking the exact type in assertIsInstance doc.
http://hg.python.org/cpython/rev/583aff635ce1

New changeset 196d485ed26d by Ezio Melotti in branch 'default':
#13387: merge with 3.2.
http://hg.python.org/cpython/rev/196d485ed26d
msg147899 - (view) Author: Ezio Melotti (ezio.melotti) * (Python committer) Date: 2011-11-18 17:01
Fixed.
msg147903 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2011-11-18 17:06
+ To check for a specific type (without including superclasses) use
+ :func:`assertIs(type(obj), cls) <assertIs>`.

Don’t you mean “without accepting subclasses”, not superclasses?
msg147906 - (view) Author: Ezio Melotti (ezio.melotti) * (Python committer) Date: 2011-11-18 17:15
> + To check for a specific type (without including superclasses) use
> + :func:`assertIs(type(obj), cls) <assertIs>`.
>
> Don’t you mean “without accepting subclasses”, not superclasses?

I mean:
>>> class MyInt(int): pass     # my specific type
...
>>> isinstance(MyInt(5), int)  # int superclass included
True
>>> type(MyInt(5)) is int      # int superclass not included
False
>>> type(MyInt(5)) is MyInt    # check for specific type
True

Do you think I should rephrase it (or maybe just remove the (...))?
msg147991 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2011-11-20 14:28
No, I’m not talking about a rephrasing, but on a full change of meaning.  I don’t understand your use of “superclasses” at all; isinstance(x, T) checks if x is an instance of T or any subclass, am I wrong?
msg147999 - (view) Author: Ezio Melotti (ezio.melotti) * (Python committer) Date: 2011-11-20 16:09
Is "To check for the exact type, use :func:`assertIs(type(obj), cls) <assertIs>`." better?
I think the problem this solves is clear enough even without mentioning sub/superclasses.
msg148040 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2011-11-21 13:46
Your latest proposal is better.  I would prefer mentioning subclasses, but don’t feel strongly about it.  One markup nit: I’d use ``code`` instead of (ab)using :func:; the doc for assertIs is just a few paragraphs above, it won’t be hard to find.
msg149816 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2011-12-19 05:07
New changeset 88aacd3541ae by Ezio Melotti in branch '2.7':
#13387: rephrase unclear sentence.
http://hg.python.org/cpython/rev/88aacd3541ae

New changeset eccb4795767b by Ezio Melotti in branch '3.2':
#13387: rephrase unclear sentence.
http://hg.python.org/cpython/rev/eccb4795767b

New changeset 064854cef999 by Ezio Melotti in branch 'default':
#13387: merge with 3.2.
http://hg.python.org/cpython/rev/064854cef999
History
Date User Action Args
2011-12-19 05:08:15ezio.melottisetstatus: open -> closed
type: behavior -> enhancement
resolution: fixed
stage: commit review -> resolved
2011-12-19 05:07:20python-devsetmessages: + msg149816
2011-11-21 13:46:39eric.araujosetstatus: closed -> open
resolution: fixed -> (no value)
messages: + msg148040

stage: resolved -> commit review
2011-11-20 16:09:15ezio.melottisetmessages: + msg147999
2011-11-20 14:28:03eric.araujosetmessages: + msg147991
2011-11-18 17:15:33ezio.melottisetmessages: + msg147906
2011-11-18 17:06:25eric.araujosetmessages: + msg147903
2011-11-18 17:01:51ezio.melottisetstatus: open -> closed
versions: + Python 2.7, Python 3.2
messages: + msg147899

assignee: docs@python -> ezio.melotti
resolution: fixed
stage: needs patch -> resolved
2011-11-18 17:01:22python-devsetnosy: + python-dev
messages: + msg147898
2011-11-12 19:12:24floxsetassignee: docs@python
type: enhancement -> behavior
components: + Documentation
title: add exact_type argument to assertIsInstance -> suggest assertIs(type(obj), cls) for exact type checking
nosy: + docs@python

messages: + msg147523
stage: needs patch
2011-11-12 18:05:47eric.snowsetnosy: + eric.snow
messages: + msg147514
2011-11-12 17:53:54pitrousetnosy: + pitrou
messages: + msg147513
2011-11-12 17:49:15michael.foordsetmessages: + msg147512
2011-11-12 15:32:20eric.araujosetmessages: + msg147504
2011-11-12 15:25:40floxsetmessages: + msg147501
2011-11-12 14:55:24eric.araujosetnosy: + eric.araujo
messages: + msg147495
2011-11-12 14:52:18floxsetfiles: + issue13387_exact_type.diff
keywords: + patch
messages: + msg147494
2011-11-12 14:50:18floxsetfiles: + grep_test_exact_type.log
2011-11-12 14:50:03floxsetfiles: + grep_test_is_instance.log
2011-11-12 13:40:59ezio.melottisetnosy: + rhettinger, ezio.melotti
messages: + msg147482
2011-11-12 12:43:22floxcreate