classification
Title: sys.modules doc entry should reflect restrictions
Type: behavior Stage: resolved
Components: Documentation Versions: Python 3.3
process
Status: closed Resolution: wont fix
Dependencies: Superseder: Eliminate PyInterpreterState.modules.
View: 28411
Assigned To: docs@python Nosy List: docs@python, eric.snow, merwok, ncoghlan
Priority: normal Keywords: patch

Created on 2011-07-25 04:49 by eric.snow, last changed 2017-09-04 18:27 by eric.snow. This issue is now closed.

Files
File name Uploaded Description Edit
sys_modules_doc.diff eric.snow, 2011-07-25 20:36 review
Messages (9)
msg141068 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2011-07-25 04:49
The sys.modules dict is a special object.  It is the only variable of the CPython interpreter object that is exposed in the sys module[1].  Everything else in sys lives in the module.  However, the modules dict lives in the interpreter object and is bound to the sys module separately.  No other variable of the interpreter object gets this treatment.

This situation sets up an unexpected behavior for sys.modules.  There are many places, mostly in Python/import.c, where the modules dict gets used and not by pulling from sys.modules.  These places use interp->modules directly[2].  So if sys.modules is re-bound, the imp module is using/reporting an out of sync modules dict.

One could argue that re-binding a module global is risky and should be avoided.  I agree.  Here is the use case that prompted me to march ahead anyway:

class BaseTest(TestCase):
    @classmethod
    def setUpClass(cls):
        cls.sysmodules = sys.modules
        sys.modules = sys.modules.copy()
    @classmethod
    def tearDownClass(cls):
        sys.modules = cls.sysmodules

I was writing some import related tests and wanted sys.modules to be returned to its initial state after each test.  I realise that Lib/test/support.py provides CleanImport and others address this, but you have to provide the module names to clean up.  This is an unfortunate hassle sometimes when several layers of imports happen during the import of the module you care about.

So the result was an exception when I tried importing an extension module, like "_sqlite3".  This is because in importdl.h the new module is added to the dict returned by PyImport_GetModuleDict(), not to the one at sys.modules.

For now I am doing the following to get the same effect:

class BaseTest(TestCase):
    @classmethod
    def setUpClass(cls):
        cls.sysmodules = sys.modules.copy()
    @classmethod
    def tearDownClass(cls):
        for name in sys.modules:
            del sys.modules[name]
        for name in cls.sysmodules:
            sys.modules[name] = cls.sysmodules[name]

However, this is less efficient, sort of.  I expect that the current direct use of interp->modules in the CPython code is [much?] more efficient than PySys_GetObject("modules") calls.

Proposal

In light of all this I recommend that either use of interp->modules be replaced by PySys_GetObject("modules") calls, or the sys module documentation[3] be updated to make clear that sys.modules should not be re-bound (in a CPython implementation detail note).  I'm guessing that the first option is right out.  The documentation addition would be just right.


[1] variables of the interpreter object found by grepping "interp->" in the CPython source:
     modules
     modules_by_index
     next
     codec_search_path
     codec_search_cache
     codec_error_registry
     codecs_initialized
     fscodec_initialized
     modules_reloading
     builtins
     sysdict
     tstate_head
     tscdump
     dlopenflags
[2] see PyImport_GetModuleDict() in Python/import.c
[3] http://docs.python.org/dev/library/sys.html#sys.modules
msg141075 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2011-07-25 07:22
+1 for making this limitation explicit. See the caveat on locals() [1] for an example of how to note this kind of restriction.

[1] http://docs.python.org/dev/library/functions.html#locals
msg141123 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2011-07-25 20:36
Would an implementation detail note be inappropriate here?  I only ask because it looks like the imp module's use of interp->modules is implementation specific.

Here's a patch for Doc/library/sys.rst that adds the note.
msg141399 - (view) Author: Éric Araujo (merwok) * (Python committer) Date: 2011-07-29 16:27
The note’s spirit is good, but I think something more concise would do.

Side note: Please don’t mix up unrelated cosmetic changes in your diffs.
msg158636 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2012-04-18 16:29
The original motivator:  http://mail.python.org/pipermail/python-dev/2011-July/112497.html
msg158639 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2012-04-18 16:34
also, issue 14615 is related to making sys.modules authoritative.
msg182259 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2013-02-17 04:09
One proposal would lead to the sys module growing descriptors:

http://mail.python.org/pipermail/python-ideas/2013-January/019075.html

In that case, sys.modules could update the underlying interp->modules.
msg191834 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2013-06-25 05:14
issue17953 addressed part of this.
msg301239 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2017-09-04 18:27
We're dropping PyInterpreterState.modules (#28411).
History
Date User Action Args
2017-09-04 18:27:53eric.snowsetstatus: open -> closed
superseder: Eliminate PyInterpreterState.modules.
messages: + msg301239

resolution: wont fix
stage: resolved
2013-06-25 05:14:51eric.snowsetmessages: + msg191834
2013-02-17 04:09:50eric.snowsetmessages: + msg182259
2012-04-18 16:34:15eric.snowsetmessages: + msg158639
2012-04-18 16:29:56eric.snowsetmessages: + msg158636
2011-07-29 16:27:49merwoksetnosy: + merwok
messages: + msg141399
2011-07-25 20:36:58eric.snowsetfiles: + sys_modules_doc.diff

assignee: docs@python
components: + Documentation
title: sys.modules gets special treatment -> sys.modules doc entry should reflect restrictions
keywords: + patch
nosy: + docs@python

messages: + msg141123
2011-07-25 07:22:39ncoghlansetmessages: + msg141075
2011-07-25 04:49:50eric.snowcreate