Repository navigation
"TypeError: Item in ``from list'' not a string" message #65919
Description
Activity
davidszotten commented
on Jun 11, 2014 davidszottenmannequinMannequinAuthor>>> __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 notstr. would be nice with a custom error message if this is a unicode string, explicitly mentioning that these must not be unicode or similarDo 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"JulianGindi commented
on Aug 28, 2014 JulianGindimannequinMannequinI'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?
davidszotten commented
on Aug 28, 2014 davidszottenmannequinMannequinAuthorafter 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'>JulianGindi commented
on Aug 28, 2014 JulianGindimannequinMannequinInteresting...I'll try to dig in and see what's going on.
davidszotten commented
on Aug 28, 2014 davidszottenmannequinMannequinAuthorfirst 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'])Thanks for the patch, David!
+ def test_fromlist_error_messages(self):
+ # Test for issue bpo-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#l2571So you can safely remove this check.
-
davidszotten commented
on Aug 29, 2014 davidszottenmannequinMannequinAuthornot 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?
bignose commented
on Dec 16, 2014 bignosemannequinMannequinIs there room for a better resolution: fix the API so that Unicode objects are accepted in the ‘fromlist’ items?
timgraham commented
on Oct 7, 2016 timgrahammannequinMannequinAs 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?
I think we can classify this one as a usability bug and improve the exception message.
24 remaining items
>>> __import__('encodings', fromlist=iter(('aliases', b'foobar'))) <module 'encodings' from '/home/serhiy/py/cpython/Lib/encodings/__init__.py'>
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.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).
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 loopThere 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 loopActually 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.
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.
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 withimport ...and if necessary, bind local names to objects off of the final module or a local name for the overall module.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.
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!
Is it worth to backport PR 4118 to 3.6?
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.
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:
bugs.python.org fields: