classification
Title: "TypeError: Item in ``from list'' not a string" message
Type: enhancement Stage: resolved
Components: Interpreter Core Versions: Python 3.7, Python 3.6, Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: Julian.Gindi, Tim.Graham, berker.peksag, bignose, brett.cannon, davidszotten@gmail.com, eric.snow, ezio.melotti, ncoghlan, python-dev, rhettinger, serhiy.storchaka, taleinat
Priority: normal Keywords: patch

Created on 2014-06-11 13:28 by davidszotten@gmail.com, last changed 2017-10-26 09:03 by serhiy.storchaka. This issue is now closed.

Files
File name Uploaded Description Edit
fromlist.patch davidszotten@gmail.com, 2014-08-28 22:59
fromlist2.patch davidszotten@gmail.com, 2014-08-29 08:29
issue21720.diff berker.peksag, 2016-10-07 21:31
issue21720_python3.diff berker.peksag, 2016-10-11 22:10 review
Pull Requests
URL Status Linked Edit
PR 4113 closed berker.peksag, 2017-10-25 02:48
PR 4118 merged serhiy.storchaka, 2017-10-25 12:54
PR 4128 merged serhiy.storchaka, 2017-10-26 08:22
Messages (42)
msg220267 - (view) Author: David Szotten (davidszotten@gmail.com) * Date: 2014-06-11 13:28
```
>>> __import__('fabric.', fromlist=[u'api'])
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
```

accidentally ended up with something like this via some module that was using `unicode_literals`. stumped me for a second until i realised that my variable was a string, but not `str`. would be nice with a custom error message if this is a unicode string, explicitly mentioning that these must not be unicode or similar
msg221027 - (view) Author: Ezio Melotti (ezio.melotti) * (Python committer) Date: 2014-06-19 21:41
Do you want to propose a patch?
I think the standard message in these cases is along the lines of "TypeError: fromlist argument X must be str, not unicode"
msg226038 - (view) Author: Julian Gindi (Julian.Gindi) * Date: 2014-08-28 19:45
I'm trying to replicate this issue. I do not get an error when I run

```
>>>  __import__('datetime', fromlist=[u'datetime'])
```

any more information you could provide to help move this issue forward?
msg226039 - (view) Author: David Szotten (davidszotten@gmail.com) * Date: 2014-08-28 20:12
after some trial and error it only appears to break for 3rd party packages (all 20 or so i happened to have installed), whereas everything i tried importing from the standard library worked fine

```
>>> __import__('requests.', fromlist=[u'get'])
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: Item in ``from list'' not a string

>>> __import__('os.', fromlist=[u'path'])
<module 'os' from '<snip (virtualenv)>/lib/python2.7/os.pyc'>
```
msg226040 - (view) Author: Julian Gindi (Julian.Gindi) * Date: 2014-08-28 20:15
Interesting...I'll try to dig in and see what's going on.
msg226046 - (view) Author: David Szotten (davidszotten@gmail.com) * Date: 2014-08-28 22:59
first ever patch to python, so advice on the patch would be appreciated

found an example in the stdlib that triggers bug (used in test):

`__import__('encodings', fromlist=[u'aliases'])`
msg226047 - (view) Author: Berker Peksag (berker.peksag) * (Python committer) Date: 2014-08-29 00:05
Thanks for the patch, David!

+    def test_fromlist_error_messages(self):
+        # Test for issue #21720: fromlist unicode error messages
+        try:
+            __import__('encodings', fromlist=[u'aliases'])
+        except TypeError as exc:
+            self.assertIn("must be str, not unicode", str(exc))

You could use assertRaises here:

    with self.assertRaises(TypeError) as cm:
        # ...

    self.assertIn('foo', str(cm.exception))


+        if (PyUnicode_Check(item)) {
+            PyErr_SetString(PyExc_TypeError,
+                            "Item in ``from list'' must be str, not unicode");
+            Py_DECREF(item);
+            return 0;
+        }

I think it would be better to improve the error message in Python/import.c:

    http://hg.python.org/cpython/file/2.7/Python/import.c#l2571

So you can safely remove this check.
msg226055 - (view) Author: David Szotten (davidszotten@gmail.com) * Date: 2014-08-29 08:29
not sure i follow. we need a different message if e.g. an integer is passed in

updated the patch to only run the unicode check for non-strings

or do you have a suggestion for an error message that works nicely in both cases?
msg232703 - (view) Author: Ben Finney (bignose) Date: 2014-12-16 02:50
Is there room for a better resolution: fix the API so that Unicode objects are accepted in the ‘fromlist’ items?
msg278252 - (view) Author: Tim Graham (Tim.Graham) * Date: 2016-10-07 16:29
As far as I can tell, this isn't an issue on Python 3. Can this be closed since Python 2 is only receiving bug fixes now?
msg278255 - (view) Author: Berker Peksag (berker.peksag) * (Python committer) Date: 2016-10-07 16:49
I think we can classify this one as a usability bug and improve the exception message.
msg278270 - (view) Author: Berker Peksag (berker.peksag) * (Python committer) Date: 2016-10-07 21:31
Here's a patch to demonstrate what I meant in msg226047.

Example from the REPL:

>>> __import__('encodings', fromlist=[u'aliases'])
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: Item in ``from list'' must be str, not unicode
msg278330 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2016-10-09 02:52
Berker's fix for Python 2.7 looks good to me.

However, Python 3 has a comparably vague error message, it's just inverted to complain about bytes rather than unicode due to the change in the native str type:

>>> __import__('encodings', fromlist=[b'aliases'])
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "<frozen importlib._bootstrap>", line 1013, in _handle_fromlist
TypeError: hasattr(): attribute name must be string

hasattr() in Python 2.7 is similarly unhelpful regarding what type it actually got when you give it something it doesn't expect.
msg278506 - (view) Author: Berker Peksag (berker.peksag) * (Python committer) Date: 2016-10-11 22:10
Good catch, thanks Nick. Here's a Python 3 version of the patch. I excluded Python/importlib.h from the patch to make review easier.
msg278513 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2016-10-12 03:21
Thanks Berker - looks good to me!

Should we file a separate issue regarding the similarly vague error message from hasattr() itself?
msg278515 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-10-12 04:43
If run Python 3 with -bb:

>>> __import__('encodings', fromlist=[b'aliases'])
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "<frozen importlib._bootstrap>", line 1000, in _handle_fromlist
BytesWarning: Comparison between bytes and string
msg278522 - (view) Author: Berker Peksag (berker.peksag) * (Python committer) Date: 2016-10-12 09:53
> Should we file a separate issue regarding the similarly vague error message from hasattr() itself?

+1 from me. It would be good to show users a user friendly message :)

> BytesWarning: Comparison between bytes and string

How about raising a TypeError if ``all(isinstance(l, str) for l in fromlist)`` is False? That would make the exception message less clearer since we can't include the "[...] not 'bytes'" part though.
msg278523 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-10-12 10:22
> How about raising a TypeError if ``all(isinstance(l, str) for l in
> fromlist)`` is False? That would make the exception message less clearer
> since we can't include the "[...] not 'bytes'" part though.

    for l in fromlist:
        if not isinstance(l, str):
            raise TypeError(...)

Note also that you can get an arbitrary error if fromlist is not a sequence.

Actually this issue doesn't look very important for Python 3, since it is 
unlikely that non-string is passed in fromlist. Unlike to Python 2 where you 
can pass unicode if just use unicode_literals. Other solution for Python 2 is 
allowing unicode in fromlist. Perhaps this would increase compatibility with 
Python 3.
msg278527 - (view) Author: Berker Peksag (berker.peksag) * (Python committer) Date: 2016-10-12 10:48
Well, I find using a for loop is a bit verbose in this case :)

In Python 3.2:

>>> __import__('encodings', fromlist=[b'aliases'])
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: Item in ``from list'' not a string

BytesWarning is there since Python 3.3 and I couldn't find any report on the tracker. I'd be fine with either committing the current patch or using pre-importlib exception message from 3.2 in 3.5+ (assuming the chance of passing a non-str item is low in Python 3)

> Other solution for Python 2 is allowing unicode in fromlist.

My preference is to improve the exception message and move on. Accepting both str and unicode would make the code harder to maintain and it would be better to avoid potential regressions in 2.7 (note that we already introduced regressions in the previous bugfix releases :))
msg278530 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2016-10-12 14:49
Right, Python 2.7 import is what it is at this point - if folks want something more modern, they can take a look at importlib2 :)

In this case, I think just reporting the first failing item is fine, and mentioning the type of that item in the error message is the most useful additional information we can provide to make things easier to debug.
msg278539 - (view) Author: Ben Finney (bignose) Date: 2016-10-12 19:37
On 12-Oct-2016, Nick Coghlan wrote:

> In this case, I think just reporting the first failing item is fine,
> and mentioning the type of that item in the error message is the
> most useful additional information we can provide to make things
> easier to debug.

Yes; also, the type expected, so the user knows what's different from
expected.

That is, the error message should say exactly what type is expected
*and* exactly what type failed that check.
msg278783 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2016-10-16 22:04
New changeset 7dd0910e8fbf by Berker Peksag in branch '2.7':
Issue #21720: Improve exception message when the type of fromlist is unicode
https://hg.python.org/cpython/rev/7dd0910e8fbf
msg278785 - (view) Author: Berker Peksag (berker.peksag) * (Python committer) Date: 2016-10-16 22:51
Thanks for the reviews! I pushed the patch for 2.7.

Nick, what do you think about the case Serhiy mentioned in msg278515? Should we fix it or is issue21720_python3.diff good to go?
msg278786 - (view) Author: Ben Finney (bignose) Date: 2016-10-16 23:09
On 16-Oct-2016, Roundup Robot wrote:

> New changeset 7dd0910e8fbf by Berker Peksag in branch '2.7':
> Issue #21720: Improve exception message when the type of fromlist is unicode
> https://hg.python.org/cpython/rev/7dd0910e8fbf

This is an improvement, but it still should IMO show *which* item
caused the error.

Can it state “Item in from list must be str, not {type}: {item!r}”?
That would show the exact item so the reader has a better chance at
finding where it came from.
msg278794 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2016-10-17 03:22
@Berker: the warning under "-bb" is a separate issue related to the handling of wildcard imports (_handle_fromlist searches for '*' and then pops it from a copy of the input list, replacing it with __all__ if that's defined)

@Ben: As a general principle, we don't give value information in type errors, since the error is structural rather than value based. The closest equivalent to us doing that that I'm aware of is the sequence index being given in str.join's TypeError:

    >>> "".join(["a", "b", b"c"])
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
    TypeError: sequence item 2: expected str instance, bytes found

By contrast, when sorting, we don't give *any* indication as to where in the sequence the problem was found or the specific values involved:

    >>> sorted(["a", "b", b"c"])
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
    TypeError: unorderable types: bytes() < str()

That doesn't make it a bad idea (as I think you're right that it would often make debugging easier), I'd just prefer to consider that as a separate question rather than introducing a one-off inconsistency with the general policy here (in particular, encountering TypeError is far more likely with str.join and sorted than it is with __import__, so it would be genuinely weird for the latter to have the most helpful error message).
msg304792 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-10-23 11:09
Is it all with this issue?
msg304868 - (view) Author: Ben Finney (bignose) Date: 2017-10-24 07:29
On 23-Oct-2017, Serhiy Storchaka wrote:
> Is it all with this issue?

I accept Nick's reasoning:

> As a general principle, we don't give value information in type
> errors, since the error is structural rather than value based.

as sufficient to reject my requested improvement to the message.

The change in <URL:https://hg.python.org/cpython/rev/7dd0910e8fbf> is
good enough, IMO.
msg304954 - (view) Author: Berker Peksag (berker.peksag) * (Python committer) Date: 2017-10-25 01:58
issue21720_python3.diff hasn't been applied. I'll convert my issue21720_python3.diff patch to a pull request. I like the format of Nick's "".join() example in msg278794. Here's my proposal for Python 3:

    f"Item {i} in 'from list' must be str, not {type(x).__name__!r}"
msg304960 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-10-25 05:41
I don't think the index in error message is needed. Unlike to str.join() which accepts arbitrary iterables of arbitrary names, the fromlist usually is a short tuple.

Interesting, what happen if the fromlist is not a list or tuple?

>>> __import__('encodings', fromlist=iter(('aliases', b'codecs')))
<module 'encodings' from '/home/serhiy/py/cpython/Lib/encodings/__init__.py'>

Import is successful because the iterator was exhausted by "'*' in fromlist".
msg304961 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-10-25 05:42
>>> __import__('encodings', fromlist=iter(('aliases', b'foobar')))
<module 'encodings' from '/home/serhiy/py/cpython/Lib/encodings/__init__.py'>
msg304967 - (view) Author: Berker Peksag (berker.peksag) * (Python committer) Date: 2017-10-25 08:34
> I don't think the index in error message is needed.

I'm fine with either format. It's ultimately up to Nick. Should I switch back to the 2.7 version?

> Import is successful because the iterator was exhausted by "'*' in fromlist".

This shouldn't be a problem in the latest version of PR 4113. While I don't think anyone would pass ``fromlist=iter(('aliases', b'foobar'))`` in real life, I can add it to the test. Let me know what do you think.
msg304969 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2017-10-25 09:03
I'm fine with the approach in the latest version of the PR - it does make "from x import y" slightly slower due to the extra error checking, but folks should be avoiding doing that in performance critical loops or functions anyway.

It would be nice if we could avoid that overhead for the import statement case, but we can't readily tell the difference between "__import__ called via syntax" and "__import__ called directly", and I don't think this is going to be performance critical enough to be worth introducing that complexity.

The question of how best to handle passing a consumable iterator as the from list would be a separate question, if we decided to do anything about it at all (the current implementation implicitly assumes the input is reiterable, but passing a non-container seems like a harder mistake to make than accidentally passing bytes instead of a string).
msg304971 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-10-25 09:34
The from import already is much slower than simple import:

$ ./python -m timeit 'from encodings import aliases'
500000 loops, best of 5: 475 nsec per loop
$ ./python -m timeit 'import encodings.aliases as aliases'
1000000 loops, best of 5: 289 nsec per loop

The latter executes only C code if the module already is imported, but the former executes Python code. It may be worth to add the C acceleration for this case too.

PR 4113 makes it yet slower:
 
$ ./python -m timeit 'from encodings import aliases'
500000 loops, best of 5: 793 nsec per loop
msg304984 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-10-25 13:04
There are other differences between Python 2.7 and Python 3. PR 4118 restores the Python 2.7 logic. It adds type checking, but its overhead is smaller.

$ ./python -m timeit 'from encodings import aliases'
500000 loops, best of 5: 542 nsec per loop

Actually in some cases (with '*') the new code is even slightly faster.

I don't know whether these differences were intentional, but all tests are passed.

The type of items in __all__ also is checked.
msg304986 - (view) Author: Tal Einat (taleinat) * (Python committer) Date: 2017-10-25 13:47
I can't say I agree that the performance here is practically insignificant. This will affect the startup time of Python process, and adding even 10% to that in some cases is significant.

In some of the larger codebases I've worked on, even simple scripts would import large portions of the system, and there would be thousands of such imports done in the process. There are "basic" utilities in the stdlib which are imported very often in different modules, so the performance of the import statement is not necessarily insignificant compared to that of actually loading the modules.

That being said I'm all for getting this in and implementing an optimization of the slower path in time for 3.7.
msg305005 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2017-10-25 17:38
As Nick said, if the overhead of an import statement is that critical, then you should NOT use the `from ... import ...` form at all and just stick with `import ...` and if necessary, bind local names to objects off of the final module or a local name for the overall module.
msg305006 - (view) Author: Tal Einat (taleinat) * (Python committer) Date: 2017-10-25 17:47
I understand that there is a workaround. I'm just thinking about the many existing large codebases where re-writing thousands of imports because of this is unlikely to be done, yet having somewhat longer process launch times would be surprising and unwanted.

Anyways I do think it's a very small price to pay for better error messages, and there's a good chance nobody will actually feel the difference, so let's definitely move forward with this.
msg305019 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2017-10-26 02:49
Serhiy's PR avoids the cryptic BytesWarning on Py3 while minimising the overhead of the new typecheck, so I've closed Berker's PR in favour of that one (which now has approved reviews from both Brett and I, so Serhiy will merge it when he's ready to do so).

Thanks for both patches!
msg305031 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-10-26 07:42
New changeset 41c56940c6edf3ea169332a6b039b6c8796f0475 by Serhiy Storchaka in branch 'master':
bpo-21720: Restore the Python 2.7 logic in handling a fromlist. (#4118)
https://github.com/python/cpython/commit/41c56940c6edf3ea169332a6b039b6c8796f0475
msg305032 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-10-26 07:43
Is it worth to backport PR 4118 to 3.6?
msg305033 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2017-10-26 08:06
Given that the automated cherry-pick failed, I'd consider a 3.6 backport nice to have, but definitely not essential.

My rationale for that is that "from __future__ import unicode_literals" makes it fairly easy to stumble over the 2.7 variant of this error message, but we're not aware of a similarly implicit way of encountering the 3.x variant.
msg305039 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-10-26 09:02
New changeset 2b5cbbb13c6c9138d04c3ca4eb7431f8c65d8e65 by Serhiy Storchaka in branch '3.6':
[3.6] bpo-21720: Restore the Python 2.7 logic in handling a fromlist. (GH-4118) (#4128)
https://github.com/python/cpython/commit/2b5cbbb13c6c9138d04c3ca4eb7431f8c65d8e65
History
Date User Action Args
2017-10-26 09:03:53serhiy.storchakasetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2017-10-26 09:02:59serhiy.storchakasetmessages: + msg305039
2017-10-26 08:22:29serhiy.storchakasetstage: backport needed -> patch review
pull_requests: + pull_request4093
2017-10-26 08:06:15ncoghlansetmessages: + msg305033
stage: patch review -> backport needed
2017-10-26 07:43:27serhiy.storchakasetmessages: + msg305032
2017-10-26 07:42:05serhiy.storchakasetmessages: + msg305031
2017-10-26 02:49:46ncoghlansetmessages: + msg305019
2017-10-25 17:47:36taleinatsetmessages: + msg305006
2017-10-25 17:38:10brett.cannonsetmessages: + msg305005
2017-10-25 13:47:43taleinatsetnosy: + taleinat
messages: + msg304986
2017-10-25 13:04:37serhiy.storchakasetmessages: + msg304984
2017-10-25 12:54:17serhiy.storchakasetpull_requests: + pull_request4088
2017-10-25 09:34:07serhiy.storchakasetmessages: + msg304971
2017-10-25 09:04:00ncoghlansetmessages: + msg304969
2017-10-25 08:34:23berker.peksagsetmessages: + msg304967
2017-10-25 05:42:14serhiy.storchakasetmessages: + msg304961
2017-10-25 05:41:10serhiy.storchakasetmessages: + msg304960
2017-10-25 02:48:48berker.peksagsetstage: needs patch -> patch review
pull_requests: + pull_request4083
2017-10-25 01:58:48berker.peksagsetstatus: closed -> open
versions: - Python 3.5
messages: + msg304954

resolution: fixed -> (no value)
stage: resolved -> needs patch
2017-10-24 08:02:32serhiy.storchakasetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2017-10-24 07:29:34bignosesetstatus: pending -> open

messages: + msg304868
2017-10-23 11:09:53serhiy.storchakasetstatus: open -> pending

messages: + msg304792
2016-10-17 03:22:24ncoghlansetmessages: + msg278794
2016-10-16 23:09:29bignosesetmessages: + msg278786
2016-10-16 22:51:30berker.peksagsetmessages: + msg278785
2016-10-16 22:04:27python-devsetnosy: + python-dev
messages: + msg278783
2016-10-12 19:37:11bignosesetmessages: + msg278539
2016-10-12 14:49:42ncoghlansetmessages: + msg278530
2016-10-12 10:48:55berker.peksagsetmessages: + msg278527
2016-10-12 10:22:15serhiy.storchakasetmessages: + msg278523
title: "TypeError: Item in ``from list'' not a string" message -> "TypeError: Item in ``from list'' not a string" message
2016-10-12 09:53:42berker.peksagsetkeywords: - easy

messages: + msg278522
2016-10-12 04:43:52serhiy.storchakasetnosy: + serhiy.storchaka
messages: + msg278515
2016-10-12 03:21:10ncoghlansetmessages: + msg278513
2016-10-11 22:10:04berker.peksagsetfiles: + issue21720_python3.diff

messages: + msg278506
components: + Interpreter Core
versions: + Python 3.5, Python 3.6, Python 3.7
2016-10-09 02:52:11ncoghlansetmessages: + msg278330
2016-10-07 21:31:29berker.peksagsetfiles: + issue21720.diff

messages: + msg278270
2016-10-07 16:49:44berker.peksagsetnosy: + ncoghlan
messages: + msg278255
2016-10-07 16:29:29Tim.Grahamsetnosy: + Tim.Graham
messages: + msg278252
2014-12-16 02:50:26bignosesetnosy: brett.cannon, rhettinger, ezio.melotti, bignose, eric.snow, berker.peksag, Julian.Gindi, davidszotten@gmail.com
2014-12-16 02:50:14bignosesetnosy: + bignose
messages: + msg232703
2014-08-29 08:29:17davidszotten@gmail.comsetfiles: + fromlist2.patch

messages: + msg226055
2014-08-29 00:05:03berker.peksagsetmessages: + msg226047
stage: needs patch -> patch review
2014-08-28 22:59:01davidszotten@gmail.comsetfiles: + fromlist.patch
keywords: + patch
messages: + msg226046
2014-08-28 20:15:36Julian.Gindisetmessages: + msg226040
2014-08-28 20:12:00davidszotten@gmail.comsetmessages: + msg226039
2014-08-28 19:45:24Julian.Gindisetnosy: + Julian.Gindi
messages: + msg226038
2014-07-04 08:16:18rhettingersetassignee: rhettinger ->
2014-07-04 06:11:12rhettingersetassignee: rhettinger

nosy: + rhettinger
2014-07-04 05:30:34berker.peksagsetnosy: + berker.peksag
2014-06-19 21:41:32ezio.melottisetnosy: + ezio.melotti
messages: + msg221027

keywords: + easy
stage: needs patch
2014-06-12 01:45:42berker.peksagsetnosy: + brett.cannon, eric.snow
2014-06-11 13:28:45davidszotten@gmail.comcreate