Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Confusing error message when None used in expressions, eg. "'NoneType' object has no attribute 'foo'" #72888

Closed
gward mannequin opened this issue Nov 15, 2016 · 8 comments
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs) pending The issue will be closed if no feedback is provided type-feature A feature request or enhancement

Comments

@gward
Copy link
Mannequin

gward mannequin commented Nov 15, 2016

BPO 28702
Nosy @brettcannon, @terryjreedy, @bitdancer, @serhiy-storchaka, @ilevkivskyi

Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

Show more details

GitHub fields:

assignee = None
closed_at = None
created_at = <Date 2016-11-15.20:04:43.057>
labels = ['interpreter-core', 'type-feature', '3.7']
title = 'Confusing error message when None used in expressions, eg. "\'NoneType\' object has no attribute \'foo\'"'
updated_at = <Date 2016-12-23.22:48:41.507>
user = 'https://bugs.python.org/gward'

bugs.python.org fields:

activity = <Date 2016-12-23.22:48:41.507>
actor = 'levkivskyi'
assignee = 'gward'
closed = False
closed_date = None
closer = None
components = ['Interpreter Core']
creation = <Date 2016-11-15.20:04:43.057>
creator = 'gward'
dependencies = []
files = []
hgrepos = []
issue_num = 28702
keywords = []
message_count = 6.0
messages = ['280884', '280885', '280892', '280893', '281160', '281429']
nosy_count = 6.0
nosy_names = ['brett.cannon', 'gward', 'terry.reedy', 'r.david.murray', 'serhiy.storchaka', 'levkivskyi']
pr_nums = []
priority = 'normal'
resolution = None
stage = 'test needed'
status = 'open'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue28702'
versions = ['Python 3.7']

@gward
Copy link
Mannequin Author

gward mannequin commented Nov 15, 2016

Python's error message when you let None accidentally sneak into an expression where it doesn't belong could be better. The canonical example is attribute lookup:

>>> a = None
>>> a.foo
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
AttributeError: 'NoneType' object has no attribute 'foo'

This assumes that the programmer knows there is only one object of type NoneType, and it is None. That's a lot to assume of a beginner, whether they are coming from another programming language ("null has a type? that's crazy talk!") or are completely new to programming ("null? none? nil? wat...??").

There are plenty of other places this use of NoneType in error messages comes up:

>>> a + 1
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: unsupported operand type(s) for +: 'NoneType' and 'int'
>>> 1 + a
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: unsupported operand type(s) for +: 'int' and 'NoneType'
>>> len(a)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: object of type 'NoneType' has no len()
>>> a < 1
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: '<' not supported between instances of 'NoneType' and 'int'

I think we can do better than this. For example, here is an proposed improvement to user experience for getting and setting attributes on None:

>>> a.foo
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
AttributeError: attempt to access attribute 'foo' of None, but None has no attributes
>>> a.foo = 42
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
AttributeError: attempt to set attribute 'foo' on None, but None is read-only

Let the bikeshedding commence. I have a working patch, but need to write tests. Will upload it here when that is done.

@gward gward mannequin self-assigned this Nov 15, 2016
@gward gward mannequin added interpreter-core (Objects, Python, Grammar, and Parser dirs) type-feature A feature request or enhancement labels Nov 15, 2016
@gward
Copy link
Mannequin Author

gward mannequin commented Nov 15, 2016

Based on a brief conversation with Brett Cannon at PyCon Canada the other day. Thanks for the idea, Brett!

@bitdancer
Copy link
Member

That presumably means adding special None support to all the places None can appear in a message, where now the code treats None like it does every other object. I'm not sure the added complexity is worth it, especially since NoneType would still creep in anywhere we'd forgotten to "fix". I'm not voting -1, but I'm dubious.

Maybe we should just make NoneType's name be 'None' :)

@serhiy-storchaka
Copy link
Member

Approximate counts.

C code:
ob_type->tp_name 161
Py_TYPE(...)->tp_name 285

Python code:
__class__.__name__ 224
__class__.__qualname__ 23
type(...).__name__ 112
type(...).__qualname__ 5

Is it worth changing about 800 places in CPython code? Not counting third-party code.

@terryjreedy
Copy link
Member

I have doubts also.

The issue is the same for NotImplemented, though the occurrence is much rarer, and similar for Ellipsis.

>>> NotImplemented.foo
Traceback (most recent call last):
  File "<pyshell#19>", line 1, in <module>
    NotImplemented.foo
AttributeError: 'NotImplementedType' object has no attribute 'foo'
>>> Ellipsis.foo
Traceback (most recent call last):
  File "<pyshell#20>", line 1, in <module>
    Ellipsis.foo
AttributeError: 'ellipsis' object has no attribute 'foo'

Replacing the type name with the object name works for this message, but not for the type errors.
TypeError: unsupported operand type(s) for +: 'None' and 'int'
is wrong.

Replacing 'NoneType' with 'None' in error messages will break code that does something like "if 'NoneType' in err.args[0]" in an exception message. The same replacement would have to be make in user code. Fortunately, it would continue to work with older versions.

@terryjreedy terryjreedy added the 3.7 (EOL) end of life label Nov 18, 2016
@gward
Copy link
Mannequin Author

gward mannequin commented Nov 22, 2016

Is it worth changing about 800 places in CPython code? Not counting third-party code.

Definitely not. My aim is not to fix every possible reference to "instance of 'NoneType'", just the handful of cases that are most frequently encountered, especially if we think they are likely to be confusing to beginners. That's why I've only modified getting and setting attributes so far; I wanted to see what the cost/benefit is like.

Renaming 'NoneType' to 'None' sounds like a much easier approach, if it works. But then saying "instance of" + tp_name comes out weird. "Instance of NoneType" is confusing if technically accurate; "instance of None" is both confusing and technically inaccurate.

Hmmmm. Still mulling.

@ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
@iritkatriel
Copy link
Member

I think changing only some of the NoneTypes to None will be confusing.

I think we should close this. It's a nice thought, but not worth the trouble.

@iritkatriel iritkatriel added pending The issue will be closed if no feedback is provided and removed 3.7 (EOL) end of life labels Oct 2, 2022
@terryjreedy
Copy link
Member

All 4 core developers were at least dubious.

@terryjreedy terryjreedy closed this as not planned Won't fix, can't repro, duplicate, stale Oct 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs) pending The issue will be closed if no feedback is provided type-feature A feature request or enhancement
Projects
None yet
Development

No branches or pull requests

4 participants