GeezerVison

Thanks for splitting this out. It was much easier to review. 😄

When you're done making the requested changes, leave the comment: I have made the requested changes; please review again.

Sorry about the churn that caused conflicts for you, @warsaw! I don't expect I'll be making any more changes to imp.py or test_imp.py before you merge this.

@warsaw, FYI, I'm moving the tests I recently added to test_imp over to test_import: gh-102561.

While I was doing that, I noticed there are other tests in test_imp that seem to be more about exercising the import system than the imp module itself, which may make it worth it to keep them. Assuming those ones really aren't imp-specific, do you know if any of them are not already covered by other tests, e.g. in test_import or test_importlib? If so, we should move them out before test_imp is deleted. I'd be glad to do so if it would help (if there are any).

All commit authors signed the Contributor License Agreement.

It seems that whoever wanted to merge main into this PR cherry-picked it instead.

The previous head was f45d6a4. I would like to leave the restoration force-push to core developers.

FYI, the actual head of this PR branch was 6159e52 . It actually turned out to be fairly straightforward to fix with some rebaseing, i.e.

git fetch upstream
git rebase upstream/main 6159e52ea7000499260d18cee833761811a67fa1

I've (force) pushed the fixed version. (Also accidentally hit the "ready for review button", which I've reversed for now).

So, grepping a bit, looks like imp is still referred to various potentially undesired places:

• A few places in the docs, including at least one spot that's causing the docs build to fail, as its resulting in a broken reference in a clean file—I can help fix those
• The stdlib_module_names.h file, which is causing the check for generated files to fail, which I'm assuming is due to Eric & Co's work and can be fixed by regenerating it via Tools/build/generate_stdlib_module_names.py — it doesn't seem to work on Windows
• Some comments in the code that still refer to it
• A couple references in the opcode generation as well
• Used in the (possibly legacy) importbench script in Tools—should that code or the whole script just be removed, or does it need to be manually converted to importlib? Not sure if its still actually used now that we have a lot of other comprehensive benchmark sources like pyperformance—the script itself mentions this (was last touched 4 years ago).

I'm assuming the test failures due to ensurepip are expected, as referred to above, so I ignored those. I also ignored the references to the very low-level C _imp module used by importlib (and previously imp).

Here's a full list, pruned for false positives and instances that should stay (NEWS, what's new, historical notes, etc):

• Doc/c-api/import.rst:189: Uses :func:`imp.source_from_cache()` in calculating the source path if
• Doc/library/functions.rst:1990: module: imp
• Doc/reference/import.rst:1078: :class:`imp.NullImporter` in the :data:`sys.path_importer_cache`. It
• Doc/tools/.nitignore:155:Doc/library/imp.rst
• Lib/pydoc.py:515: (object.__name__ in ('errno', 'exceptions', 'gc', 'imp',
• Lib/test/test_importlib/util.py:134: if name in ('sys', 'marshal', 'imp'):
• Modules/_testmultiphase.c:889:/*** Helper for imp test ***/
• Python/import.c:594: for the first time or via imp.load_dynamic().
• Python/import.c:596: Here's a summary, using imp.load_dynamic() as the starting point:
• Python/import.c:598: 1. imp.load_dynamic() -> importlib._bootstrap._load()
• Python/import.c:3789:"(Extremely) low-level import machinery bits as used by importlib and imp.");
• Python/makeopcodetargets.py:13: import imp
• Python/makeopcodetargets.py:20: return imp.load_module(modname, *imp.find_module(modname[modpath]))
• Python/pylifecycle.c:2172: /* Main is a little special - imp.is_builtin("__main__") will return
• Python/stdlib_module_names.h:167:"imp",
• Tools/c-analyzer/TODO:498:Python/import.c:PyImport_ReloadModule():PyId_imp _Py_IDENTIFIER(imp)
• Tools/importbench/importbench.py:9:import imp
• Tools/importbench/importbench.py:43: module = imp.new_module(name)
• Tools/importbench/importbench.py:68: assert not os.path.exists(imp.cache_from_source(mapping[name]))
• Tools/importbench/importbench.py:83: bytecode_path = imp.cache_from_source(module.__file__)
• Tools/importbench/importbench.py:111: os.unlink(imp.cache_from_source(mapping[name]))
• Tools/importbench/importbench.py:113: assert not os.path.exists(imp.cache_from_source(mapping[name]))
• Tools/importbench/importbench.py:124: os.unlink(imp.cache_from_source(module.__file__))
• Tools/importbench/importbench.py:144: assert os.path.exists(imp.cache_from_source(mapping[name]))

Also, not totally sure why, but when I build this commit, I get this delta in the generated files which I cannot reproduce on the main parent commit, adding zipimporter a few places:

diff --git a/Include/internal/pycore_global_objects_fini_generated.h b/Include/internal/pycore_global_objects_fini_generated.h
index 14dfd9ea582..d79b56574f7 100644
--- a/Include/internal/pycore_global_objects_fini_generated.h
+++ b/Include/internal/pycore_global_objects_fini_generated.h
@@ -1232,6 +1232,7 @@ _PyStaticObjects_CheckRefcnt(PyInterpreterState *interp) {
     _PyStaticObject_CheckRefcnt((PyObject *)&_Py_ID(x));
     _PyStaticObject_CheckRefcnt((PyObject *)&_Py_ID(year));
     _PyStaticObject_CheckRefcnt((PyObject *)&_Py_ID(zdict));
+    _PyStaticObject_CheckRefcnt((PyObject *)&_Py_ID(zipimporter));
     _PyStaticObject_CheckRefcnt((PyObject *)&_Py_SINGLETON(strings).ascii[0]);
     _PyStaticObject_CheckRefcnt((PyObject *)&_Py_SINGLETON(strings).ascii[1]);
     _PyStaticObject_CheckRefcnt((PyObject *)&_Py_SINGLETON(strings).ascii[2]);
diff --git a/Include/internal/pycore_global_strings.h b/Include/internal/pycore_global_strings.h
index 6f430bb25eb..77169765294 100644
--- a/Include/internal/pycore_global_strings.h
+++ b/Include/internal/pycore_global_strings.h
@@ -718,6 +718,7 @@ struct _Py_global_strings {
         STRUCT_FOR_ID(x)
         STRUCT_FOR_ID(year)
         STRUCT_FOR_ID(zdict)
+        STRUCT_FOR_ID(zipimporter)
     } identifiers;
     struct {
         PyASCIIObject _ascii;
diff --git a/Include/internal/pycore_runtime_init_generated.h b/Include/internal/pycore_runtime_init_generated.h
index 0452c4c6155..82014a8b10d 100644
--- a/Include/internal/pycore_runtime_init_generated.h
+++ b/Include/internal/pycore_runtime_init_generated.h
@@ -1224,6 +1224,7 @@ extern "C" {
     INIT_ID(x), \
     INIT_ID(year), \
     INIT_ID(zdict), \
+    INIT_ID(zipimporter), \
 }

 #define _Py_str_ascii_INIT { \
diff --git a/Include/internal/pycore_unicodeobject_generated.h b/Include/internal/pycore_unicodeobject_generated.h
index 7114a5416f2..6d4dcc229aa 100644
--- a/Include/internal/pycore_unicodeobject_generated.h
+++ b/Include/internal/pycore_unicodeobject_generated.h
@@ -2007,6 +2007,9 @@ _PyUnicode_InitStaticStrings(PyInterpreterState *interp) {
     string = &_Py_ID(zdict);
     assert(_PyUnicode_CheckConsistency(string, 1));
     _PyUnicode_InternInPlace(interp, &string);
+    string = &_Py_ID(zipimporter);
+    assert(_PyUnicode_CheckConsistency(string, 1));
+    _PyUnicode_InternInPlace(interp, &string);
 }
 /* End auto-generated code */
 #ifdef __cplusplus

@warsaw, FYI, I'm moving the tests I recently added to test_imp over to test_import: gh-102561.

While I was doing that, I noticed there are other tests in test_imp that seem to be more about exercising the import system than the imp module itself, which may make it worth it to keep them. Assuming those ones really aren't imp-specific, do you know if any of them are not already covered by other tests, e.g. in test_import or test_importlib? If so, we should move them out before test_imp is deleted. I'd be glad to do so if it would help (if there are any).

I missed this comment earlier but yes, we should audit test_imp.py for general import system tests and move them over. I plan on working on this at Pycon.

So, grepping a bit, looks like imp is still referred to various potentially undesired places

Thanks for doing this audit @CAM-Gerlach !

The imp module is deprecated for 9 years, since Python 3.4 (2014). Problem: a search for (from imp\b|import imp\b) regex in PyPI top 5,000 projects match 153 projects (260 lines).

That's why I didn't propose a PR to remove it in Python 3.12.

It would be nice for the Python community to collaborate with these projects to reduce the number of impacted projects before removing the imp module.

Example of projects:

• ansible-7.4.0
• boto-2.49.0
• cffi-1.15.1
• conda-4.3.16
• django-compat-1.0.15
• eventlet-0.33.3
• lxml-4.9.2
• matplotlib-3.7.1
• mercurial-6.4.1
• nose-1.3.7
• Paste-3.5.2
• pip-23.0.1
• pipenv-2023.3.20
• reportlab-3.6.12
• setuptools-67.6.1
• Twisted-22.10.0
• virtualenv-20.21.0

Full list of projects:

pip and setuptools:

PYPI-2023-04-13/pip-23.0.1.tar.gz: pip-23.0.1/src/pip/_vendor/distlib/wheel.py: import imp
PYPI-2023-04-13/pip-23.0.1.tar.gz: pip-23.0.1/src/pip/_vendor/pkg_resources/__init__.py: import imp as _imp
PYPI-2023-04-13/setuptools-67.6.1.tar.gz: setuptools-67.6.1/pkg_resources/__init__.py: import imp as _imp