ImportError raised for a circular import
Recently I fell into the trap of creating a circular import and yet again it took time to figure out what was wrong. I'm wondering why the python import code does not detect this error and raise an exception. I took a look at the code and got as far as figuring out that I would need to add the detection to the python 3 import code. Unless I missed something I cannot get the detection without modifying the core code as I could see no way to hook the process cleanly. Is it reasonable idea to add this detection to python? I am willing to work on a patch. Barry
But circular imports are sometimes needed in modules. For example when you have two classes in two different modules that reference each other in their methods (and because you can't pre-declare classes like in C++). 2017-06-13 20:30 GMT+02:00 Barry Scott <barry@barrys-emacs.org>:
-- Antoine Rozo
On Wed, Jun 14, 2017 at 6:35 AM, Barry <barry@barrys-emacs.org> wrote:
Depends on your definition of "circular". Consider this: # __init__.py from flask import Flask app = Flask(__name__) from . import views # views.py from . import app @app.route("/") def home(): ... Technically this is circular. During the loading of __init__, views will be imported, which then imports something from __init__. But it's perfectly well-defined (there's no way that views will ever be the first one imported, per the rules of packages) and it makes good sense. An error on circular imports, or even a warning, would be very annoying here. ChrisA
I didn't interpret the initial email as wanting an error on *all* circular imports. Merely those which are unresolvable. I've definitely helped people diagnose circular imports and wished there was an error that called that out programmatically, even if it's just a string admonition to check for circular imports, appended to the ImportError message. On Tue, Jun 13, 2017 at 1:43 PM, Chris Angelico <rosuav@gmail.com> wrote:
On Wed, Jun 14, 2017 at 8:10 AM, Mahmoud Hashemi <mahmoud@hatnote.com> wrote:
Oh! That could be interesting. How about a traceback in the import chain? # a.py import b q = 1 # b.py import c # c.py from a import q c.py will trigger an ImportError, but that could say something like... oh look: $ python3 a.py Traceback (most recent call last): File "a.py", line 1, in <module> import b File "/home/rosuav/tmp/asdf/b.py", line 1, in <module> import c File "/home/rosuav/tmp/asdf/c.py", line 1, in <module> from a import q ImportError: cannot import name 'q' from 'a' (/home/rosuav/tmp/asdf/a.py) Already happens :) A bit harder, but also possible, would be to have an AttributeError on a module recognize that an import is happening, and report a possible circular import. That'd take some engineering, but it would be helpful. That'd catch cases like: # c.py import a print(a.q) Is that what you're looking for? ChrisA
Oh I know the traceback, I've had many brought to my desk by a confused junior dev, looking a lot like yours truly a few years back. :) My only problem with it is that it makes people look at "a.py". And if you look at "a.py", you'll see there's a "q" there. While most of us on this list will check for the circular import, it's quite a bit of headscratching as to why Python can't find the "q", when it's right there in the file. Calling out the circular import possibility explicitly makes people look at the _whole_ stack track, and even if real stack traces are quite a bit longer, they'll probably make the connection. The AttributeError idea is definitely interesting because it's also a major player in circular import confusions. I think it's an ambitious idea, and would be very exciting if it were implemented. On Tue, Jun 13, 2017 at 3:36 PM, Chris Angelico <rosuav@gmail.com> wrote:
On 14 June 2017 at 08:49, Mahmoud Hashemi <mahmoud@hatnote.com> wrote:
Oh I know the traceback, I've had many brought to my desk by a confused junior dev, looking a lot like yours truly a few years back. :)
Something worth noting is that as of 3.7, all circular imports that actually *are* resolvable at runtime will be resolved: https://bugs.python.org/issue30024 However, that only impacts submodules where the submodule entry exists in sys.modules, but the name hasn't been bound in the parent module yet - it doesn't help with module level attributes that would be defined eventually, but we're still too early in the module's import process for them to exist yet. As Chris pointed out, there are two key points of name resolution to take into account for those cases: * ModuleType.__getattr__ ("import a; a.q") * from_list processing in the import system ("from a import q") Since the import system already keeps track of "currently in progress imports" to manage the per-module import locks, both of those could potentially be updated to query _frozen_importlib._module_locks to find out if the source module was currently in the process of being imported and raise a new CircularImportError that inherited from both AttributeError and ImportError when that was the case. Cheers, Nick. -- Nick Coghlan | ncoghlan@gmail.com | Brisbane, Australia
On 14 June 2017 at 13:02, Mahmoud Hashemi <mahmoud@hatnote.com> wrote:
The `IMPORT_FROM` opcode's error handling would probably be the best place to start poking around: https://github.com/python/cpython/blob/master/Python/ceval.c#L5055 If you can prove the concept there, that would: 1. Directly handle the "from x import y" and "import x.y as name" cases 2. Provide a starting point for factoring out a "report missing module attribute" helper that could be shared with ModuleType As an example of querying _frozen_importlib state from C code, I'd point to https://github.com/python/cpython/blob/master/Python/import.c#L478 Cheers, Nick. P.S. I also double checked that ImportError & AttributeError have compatible binary layouts, so dual inheritance from them works :) -- Nick Coghlan | ncoghlan@gmail.com | Brisbane, Australia
I had thought that the solution would be in the .py implementation of the import machinery not in the core C code. I was going to simply keep track of the names of the modules that are being imported and raise an exception if an import attempted to import a module that had not completed being imported. It seems from a quick loom at the code that this would be practical. Are you saying that there is a subtle point about import and detection of cycles that means the work must be done in C? Barry
It seemed that PyImport_ImportModuleLevelObject() always calls out the interp->importlib. For example: value = _PyObject_CallMethodIdObjArgs(interp->importlib, &PyId__lock_unlock_module, abs_name, NULL); Where interp->importlib is the frozen importlib.py code I thought. I'd assumed that I would need to change the importlib.py code and build that as the frozen version to implement this. Barry
Barry, that kind of circular import is actually fine in many (if not most) cases. Modules are immediately created and importable, thenincrementally populated. The problem arises when you try to do something with contents of the module that have not been populated, usually manifesting in the AttributeError above. If you'd like to test this yourself, I've made a tiny demo with a little bit of documentation: https://gist.github.com/mahmoud/32fd056a3d4d1cd03a4e8aeff6b5ee70 Long story short, circular imports can be a code smell, but they're by no means universally an error condition. :) On Sun, Jun 18, 2017 at 11:21 AM, Barry Scott <barry@barrys-emacs.org> wrote:
On 19 June 2017 at 04:47, Mahmoud Hashemi <mahmoud@hatnote.com> wrote:
Indeed, and this is why we've been begrudgingly giving ground and ensuring that the cases that *can* be made to work actually succeed in practice. We'll let code linters complain about those cycles, rather than having the interpreter continue to get confused :) However, permitting resolvable circular imports means that for the remaining cases that are genuinely irresolvably broken, any custom detection logic needs to live in some combination of the module attribute lookup code and the IMPORT_FROM opcode implementation, rather than being able to isolate the changes to the import machinery itself. Cheers, Nick. -- Nick Coghlan | ncoghlan@gmail.com | Brisbane, Australia
On Sun, Jun 18, 2017 at 07:21:21PM +0100, Barry Scott <barry@barrys-emacs.org> wrote:
Please don't do that. In SQLObject module dbconnection imports connection modules to run registration code in their __init__; connection modules import name DBAPI from dbconnection. To avoid problems with circular import dbconnection imports connection modules at the very end. I'm afraid you plan if implemented would prevent me from doing that. See the code. dbconnection.py: https://github.com/sqlobject/sqlobject/blob/master/sqlobject/dbconnection.py... Connection modules (just a few examples): https://github.com/sqlobject/sqlobject/blob/master/sqlobject/mysql/mysqlconn... https://github.com/sqlobject/sqlobject/blob/master/sqlobject/postgres/pgconn... Oleg. -- Oleg Broytman http://phdru.name/ phd@phdru.name Programmers don't die, they just GOSUB without RETURN.
On 13 June 2017 at 23:36, Chris Angelico <rosuav@gmail.com> wrote:
I have a feeling that mypy might flag circular imports. I've not used mypy myself, but I just saw the output from a project where we enabled very basic use of mypy (no type hints at all, yet) and saw an error reported about a circular import. So with suitable configuration, mypy could help here (and may lead to other benefits if you want to use more of its capabilities). OTOH, having the interpreter itself flag that it had got stuck in an import loop with an explicit message explaining the problem sounds like a reasonable idea. Paul
On 14 June 2017 at 09:59, Paul Moore <p.f.moore@gmail.com> wrote:
Mypy doesn't always flag invalid circular imports, there is an old issue about this, see https://github.com/python/mypy/issues/61 But yes, one gets many other benefits like static type checking (including checking the types of things imported in a circular manner). -- Ivan
I've also been thinking about this lately. I can remember being confused the first time I saw "ImportError: cannot import name X". As there are multiple things that can cause this error, it took me a while to find a stackoverflow post that suggested that this might be due to circular imports. After learning this, I still had to read a few sources to understand what circular imports were and how to fix the problem. A quick stackoverflow search reveals that people frequently have questions about this error message: https://www.google.com/search?q=stackoverflow+python+import+error&oq=stackoverflow+python+import+error&aqs=chrome..69i57j0j69i64.6423j0j7&sourceid=chrome&ie=UTF-8#q=site:stackoverflow.com+python+importerror+%22cannot+import+name%22 At the very least, it would be nice if the error message could differentiate between different causes for this error. Ideally, however, I'd love if for circular imports, it included text on what they are and how to resolve them. On Tue, Jun 13, 2017 at 6:10 PM, Mahmoud Hashemi <mahmoud@hatnote.com> wrote:
Huh? The semantics of node.js seem to be completely similar to Python in this respect. In both cases the circular import works if you go through the mutable module object but fails if both modules circularly try to import module members directly. Stephan Op 14 jun. 2017 11:17 a.m. schreef "Abdur-Rahmaan Janhangeer" < arj.python@gmail.com>:
But circular imports are sometimes needed in modules. For example when you have two classes in two different modules that reference each other in their methods (and because you can't pre-declare classes like in C++). 2017-06-13 20:30 GMT+02:00 Barry Scott <barry@barrys-emacs.org>:
-- Antoine Rozo
On Wed, Jun 14, 2017 at 6:35 AM, Barry <barry@barrys-emacs.org> wrote:
Depends on your definition of "circular". Consider this: # __init__.py from flask import Flask app = Flask(__name__) from . import views # views.py from . import app @app.route("/") def home(): ... Technically this is circular. During the loading of __init__, views will be imported, which then imports something from __init__. But it's perfectly well-defined (there's no way that views will ever be the first one imported, per the rules of packages) and it makes good sense. An error on circular imports, or even a warning, would be very annoying here. ChrisA
I didn't interpret the initial email as wanting an error on *all* circular imports. Merely those which are unresolvable. I've definitely helped people diagnose circular imports and wished there was an error that called that out programmatically, even if it's just a string admonition to check for circular imports, appended to the ImportError message. On Tue, Jun 13, 2017 at 1:43 PM, Chris Angelico <rosuav@gmail.com> wrote:
On Wed, Jun 14, 2017 at 8:10 AM, Mahmoud Hashemi <mahmoud@hatnote.com> wrote:
Oh! That could be interesting. How about a traceback in the import chain? # a.py import b q = 1 # b.py import c # c.py from a import q c.py will trigger an ImportError, but that could say something like... oh look: $ python3 a.py Traceback (most recent call last): File "a.py", line 1, in <module> import b File "/home/rosuav/tmp/asdf/b.py", line 1, in <module> import c File "/home/rosuav/tmp/asdf/c.py", line 1, in <module> from a import q ImportError: cannot import name 'q' from 'a' (/home/rosuav/tmp/asdf/a.py) Already happens :) A bit harder, but also possible, would be to have an AttributeError on a module recognize that an import is happening, and report a possible circular import. That'd take some engineering, but it would be helpful. That'd catch cases like: # c.py import a print(a.q) Is that what you're looking for? ChrisA
Oh I know the traceback, I've had many brought to my desk by a confused junior dev, looking a lot like yours truly a few years back. :) My only problem with it is that it makes people look at "a.py". And if you look at "a.py", you'll see there's a "q" there. While most of us on this list will check for the circular import, it's quite a bit of headscratching as to why Python can't find the "q", when it's right there in the file. Calling out the circular import possibility explicitly makes people look at the _whole_ stack track, and even if real stack traces are quite a bit longer, they'll probably make the connection. The AttributeError idea is definitely interesting because it's also a major player in circular import confusions. I think it's an ambitious idea, and would be very exciting if it were implemented. On Tue, Jun 13, 2017 at 3:36 PM, Chris Angelico <rosuav@gmail.com> wrote:
On 14 June 2017 at 08:49, Mahmoud Hashemi <mahmoud@hatnote.com> wrote:
Oh I know the traceback, I've had many brought to my desk by a confused junior dev, looking a lot like yours truly a few years back. :)
Something worth noting is that as of 3.7, all circular imports that actually *are* resolvable at runtime will be resolved: https://bugs.python.org/issue30024 However, that only impacts submodules where the submodule entry exists in sys.modules, but the name hasn't been bound in the parent module yet - it doesn't help with module level attributes that would be defined eventually, but we're still too early in the module's import process for them to exist yet. As Chris pointed out, there are two key points of name resolution to take into account for those cases: * ModuleType.__getattr__ ("import a; a.q") * from_list processing in the import system ("from a import q") Since the import system already keeps track of "currently in progress imports" to manage the per-module import locks, both of those could potentially be updated to query _frozen_importlib._module_locks to find out if the source module was currently in the process of being imported and raise a new CircularImportError that inherited from both AttributeError and ImportError when that was the case. Cheers, Nick. -- Nick Coghlan | ncoghlan@gmail.com | Brisbane, Australia
On 14 June 2017 at 13:02, Mahmoud Hashemi <mahmoud@hatnote.com> wrote:
The `IMPORT_FROM` opcode's error handling would probably be the best place to start poking around: https://github.com/python/cpython/blob/master/Python/ceval.c#L5055 If you can prove the concept there, that would: 1. Directly handle the "from x import y" and "import x.y as name" cases 2. Provide a starting point for factoring out a "report missing module attribute" helper that could be shared with ModuleType As an example of querying _frozen_importlib state from C code, I'd point to https://github.com/python/cpython/blob/master/Python/import.c#L478 Cheers, Nick. P.S. I also double checked that ImportError & AttributeError have compatible binary layouts, so dual inheritance from them works :) -- Nick Coghlan | ncoghlan@gmail.com | Brisbane, Australia
I had thought that the solution would be in the .py implementation of the import machinery not in the core C code. I was going to simply keep track of the names of the modules that are being imported and raise an exception if an import attempted to import a module that had not completed being imported. It seems from a quick loom at the code that this would be practical. Are you saying that there is a subtle point about import and detection of cycles that means the work must be done in C? Barry
It seemed that PyImport_ImportModuleLevelObject() always calls out the interp->importlib. For example: value = _PyObject_CallMethodIdObjArgs(interp->importlib, &PyId__lock_unlock_module, abs_name, NULL); Where interp->importlib is the frozen importlib.py code I thought. I'd assumed that I would need to change the importlib.py code and build that as the frozen version to implement this. Barry
Barry, that kind of circular import is actually fine in many (if not most) cases. Modules are immediately created and importable, thenincrementally populated. The problem arises when you try to do something with contents of the module that have not been populated, usually manifesting in the AttributeError above. If you'd like to test this yourself, I've made a tiny demo with a little bit of documentation: https://gist.github.com/mahmoud/32fd056a3d4d1cd03a4e8aeff6b5ee70 Long story short, circular imports can be a code smell, but they're by no means universally an error condition. :) On Sun, Jun 18, 2017 at 11:21 AM, Barry Scott <barry@barrys-emacs.org> wrote:
On 19 June 2017 at 04:47, Mahmoud Hashemi <mahmoud@hatnote.com> wrote:
Indeed, and this is why we've been begrudgingly giving ground and ensuring that the cases that *can* be made to work actually succeed in practice. We'll let code linters complain about those cycles, rather than having the interpreter continue to get confused :) However, permitting resolvable circular imports means that for the remaining cases that are genuinely irresolvably broken, any custom detection logic needs to live in some combination of the module attribute lookup code and the IMPORT_FROM opcode implementation, rather than being able to isolate the changes to the import machinery itself. Cheers, Nick. -- Nick Coghlan | ncoghlan@gmail.com | Brisbane, Australia
On Sun, Jun 18, 2017 at 07:21:21PM +0100, Barry Scott <barry@barrys-emacs.org> wrote:
Please don't do that. In SQLObject module dbconnection imports connection modules to run registration code in their __init__; connection modules import name DBAPI from dbconnection. To avoid problems with circular import dbconnection imports connection modules at the very end. I'm afraid you plan if implemented would prevent me from doing that. See the code. dbconnection.py: https://github.com/sqlobject/sqlobject/blob/master/sqlobject/dbconnection.py... Connection modules (just a few examples): https://github.com/sqlobject/sqlobject/blob/master/sqlobject/mysql/mysqlconn... https://github.com/sqlobject/sqlobject/blob/master/sqlobject/postgres/pgconn... Oleg. -- Oleg Broytman http://phdru.name/ phd@phdru.name Programmers don't die, they just GOSUB without RETURN.
On 13 June 2017 at 23:36, Chris Angelico <rosuav@gmail.com> wrote:
I have a feeling that mypy might flag circular imports. I've not used mypy myself, but I just saw the output from a project where we enabled very basic use of mypy (no type hints at all, yet) and saw an error reported about a circular import. So with suitable configuration, mypy could help here (and may lead to other benefits if you want to use more of its capabilities). OTOH, having the interpreter itself flag that it had got stuck in an import loop with an explicit message explaining the problem sounds like a reasonable idea. Paul
On 14 June 2017 at 09:59, Paul Moore <p.f.moore@gmail.com> wrote:
Mypy doesn't always flag invalid circular imports, there is an old issue about this, see https://github.com/python/mypy/issues/61 But yes, one gets many other benefits like static type checking (including checking the types of things imported in a circular manner). -- Ivan
I've also been thinking about this lately. I can remember being confused the first time I saw "ImportError: cannot import name X". As there are multiple things that can cause this error, it took me a while to find a stackoverflow post that suggested that this might be due to circular imports. After learning this, I still had to read a few sources to understand what circular imports were and how to fix the problem. A quick stackoverflow search reveals that people frequently have questions about this error message: https://www.google.com/search?q=stackoverflow+python+import+error&oq=stackoverflow+python+import+error&aqs=chrome..69i57j0j69i64.6423j0j7&sourceid=chrome&ie=UTF-8#q=site:stackoverflow.com+python+importerror+%22cannot+import+name%22 At the very least, it would be nice if the error message could differentiate between different causes for this error. Ideally, however, I'd love if for circular imports, it included text on what they are and how to resolve them. On Tue, Jun 13, 2017 at 6:10 PM, Mahmoud Hashemi <mahmoud@hatnote.com> wrote:
Huh? The semantics of node.js seem to be completely similar to Python in this respect. In both cases the circular import works if you go through the mutable module object but fails if both modules circularly try to import module members directly. Stephan Op 14 jun. 2017 11:17 a.m. schreef "Abdur-Rahmaan Janhangeer" < arj.python@gmail.com>:
participants (12)
-
Abdur-Rahmaan Janhangeer -
Antoine Rozo -
Barry -
Barry Scott
-
Chris Angelico -
Ivan Levkivskyi -
Mahmoud Hashemi -
Matt -
Nick Coghlan -
Oleg Broytman -
Paul Moore -
Stephan Houben