Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve the docs regarding the migration from imp to importlib #104212

Closed
alexprengere opened this issue May 5, 2023 · 19 comments
Closed

Improve the docs regarding the migration from imp to importlib #104212

alexprengere opened this issue May 5, 2023 · 19 comments
Labels
3.11 only security fixes 3.12 bugs and security fixes docs Documentation in the Doc dir

Comments

@alexprengere
Copy link
Contributor

alexprengere commented May 5, 2023

Now that the imp removal has landed on main, users migrating to Python3.12 will likely need help to move to importlib.
The current imp docs have tips on how to do just that. Great!

One caveat: the imp.load_source has been removed from the docs a long time ago, now it is only visible is the Python2 version of the docs. So users of imp.load_source cannot rely on the docs to help them migrate, and have to Google this. The first results on stackoverflow are a bit wrong:

  • many point to SourceFileLoader(...).load_module(), but this is also deprecated and slated for removal in 3.12 (according to the warning)
  • some solutions point to importlib.util.spec_from_file_location, but this does not work with files that do not end with ".py"

The solution that I think is the best translation:

def imp_load_source(module_name, module_path):
    loader = SourceFileLoader(module_name, module_path)
    module = types.ModuleType(loader.name)
    loader.exec_module(module)
    return module

I think it would be beneficial to have that kind of information in the docs. Unfortunately, imp.load_source is not officially documented, but there are several GitHub issues and SO threads discussing how to migrate code to importlib. IMO, we should do one of:

  • add it back to the docs, with explanations on how to migrate it to importlib
  • just document the migration in the release notes, in the section "Porting to Python 3.12"

What do you think?

Linked PRs

@alexprengere alexprengere added the docs Documentation in the Doc dir label May 5, 2023
@warsaw
Copy link
Member

warsaw commented May 5, 2023

Maybe we need a porting guide in the importlib docs for porting older imp and other APIs to importlib? I don't like that imp module has been deleted but the docs are still there.

@hugovk
Copy link
Member

hugovk commented May 5, 2023

Good idea!

In fact we were just discussing this earlier today and thought about putting a migration guide in both the imp and importlib docs and in the 3.12 porting guide.

cc @vstinner

@hugovk hugovk added 3.11 only security fixes 3.12 bugs and security fixes labels May 5, 2023
@terryjreedy
Copy link
Member

In current 3.12, 'imp' is no longer present in the module index, which suggests that it is gone.

@vstinner
Copy link
Member

vstinner commented May 5, 2023

I don't know the importlib design, so maybe it doesn't make sense, but... Would it make sense to add a imp_load_source() function to importlib? Maybe under a different name.

@vstinner
Copy link
Member

vstinner commented May 5, 2023

My own use case of load_module(): old code written first for Python 2 using imp then ported to SourceLoader. It's a short script which looks for "test_xxx.py" files and then run them. Stupid but simple pytest-like, good enough for my needs: https://github.com/vstinner/hachoir/blob/0a030e3c8045441a0c30a04475a94f3aadd818ed/runtests.py#L72

I was already annoyed to have to port to them a first date to get rid of imp. So the code should be updated again?

@CAM-Gerlach
Copy link
Member

Would it make sense to add a imp_load_source() function to importlib?

Just to note, see, e.g., #58756 for some previous discussion of this.

vstinner added a commit to vstinner/hachoir that referenced this issue May 24, 2023
Don't use removed importlib load_modules() method.

See: python/cpython#104212
@vstinner
Copy link
Member

vstinner commented May 24, 2023

The recipe didn't work for me:

  • (1) The module has no __file__ variable
  • (2) The module was no registered in sys.modules

imp.load_source() defines __file__ and adds the module to sys.modules.

I used this recipe instead:

def load_module(module_name, filename):
    loader = importlib.machinery.SourceFileLoader(module_name, filename)
    module = types.ModuleType(loader.name)
    module.__file__ = filename
    sys.modules[module.__name__] = module
    loader.exec_module(module)
    return module

@vstinner
Copy link
Member

Minor step forward: I documented how to replace removed imp.new_module(): 457a459

@brettcannon
Copy link
Member

  • some solutions point to importlib.util.spec_from_file_location, but this does not work with files that do not end with ".py"

That's actually only true if you don't pass in the loader you need to use as the code has to guess as what you're after. You can use either importlib.util.spec_from_loader() or importlib.util.spec_from_file_location(), they just need different specifics to do the right thing.

vstinner added a commit to vstinner/cpython that referenced this issue Jun 16, 2023
vstinner added a commit to vstinner/cpython that referenced this issue Jun 19, 2023
vstinner added a commit to vstinner/cpython that referenced this issue Jun 19, 2023
Explain in What's New in Python 3.12 how to port existing code using
the removed imp to the importlib module.
vstinner added a commit to vstinner/cpython that referenced this issue Jun 19, 2023
Explain in What's New in Python 3.12 how to port existing code using
the removed imp to the importlib module.
vstinner added a commit to vstinner/cpython that referenced this issue Jun 19, 2023
Explain in What's New in Python 3.12 how to port existing code using
the removed imp to the importlib module.
@vstinner
Copy link
Member

Each time that I have to dig into importlib internals, I'm lost, so I took some notes at: https://pythondev.readthedocs.io/import.html

vstinner added a commit to vstinner/cpython that referenced this issue Jun 19, 2023
Explain in What's New in Python 3.12 how to port existing code using
the removed imp to the importlib module.
@vstinner
Copy link
Member

I'm not sure that this fix was correct in the gsutil-mirrors/boto project: replace imp.find_module() with importlib.machinery.PathFinder().find_spec() (ok), and replace imp.load_module() with importlib.util.module_from_spec(spec): here there is a missing exec_module() call, no?

@vstinner
Copy link
Member

I enhanced the documentation on how to port existing code from imp to importlib: commit 7a56a41

@brettcannon
Copy link
Member

replace imp.find_module() with importlib.machinery.PathFinder().find_spec() (ok), and replace imp.load_module() with importlib.util.module_from_spec(spec): here there is a missing exec_module() call, no?

Correct, you need to find the spec and then once you have it you can use it to load the module. If someone called find_spec() and it succeeded, then they should use the loader from that spec to import the code. See https://docs.python.org/3/library/importlib.html#approximating-importlib-import-module for details.

vstinner added a commit to vstinner/cpython that referenced this issue Jun 20, 2023
Explain how to port removed imp "load" functions to Python 3.13 in
What's New in Python 3.12.
@vstinner
Copy link
Member

Rather than having this "porting guide" in What's New in Python 3.12, maybe it can/should be moved to the importlib documentation directly?

The subprocess documentation explains how to port code from removed functions like popen2.Popen3: https://docs.python.org/dev/library/subprocess.html#replacing-os-popen-os-popen2-os-popen3

@brettcannon
Copy link
Member

Rather than having this "porting guide" in What's New in Python 3.12, maybe it can/should be moved to the importlib documentation directly?

It could be, but I think it should get removed once 3.11 reaches EOL.

The subprocess documentation explains how to port code from removed functions like popen2.Popen3: https://docs.python.org/dev/library/subprocess.html#replacing-os-popen-os-popen2-os-popen3

That should probably be removed at this point as no one knows what those modules even do anymore.

vstinner added a commit to vstinner/cpython that referenced this issue Jun 21, 2023
Explain how to port removed imp.load_source() to importlib in What's
New in Python 3.12.
@vstinner
Copy link
Member

I proposed PR #105951 to add recipes to replace the following removed imp functions: load_source(), load_compiled(), load_dynamic(), load_package(), init_builtin(). These functions are special because you can specify a path from where the module is loaded. These functions are like "implementation details" of load_module() which should be used with find_module().

To compare it to importlib design, find_module() produces something like an importlib "spec" (ModuleSpec), and load_module() implements different importlib "loaders" depending on the module type. The problem is that there is no 1-to-1 mapping between imp and importlib since these two modules have very different design on purpose.

I'm not comfortable with adding such long wall of text just to update code using imp in What's New in Python 3.12. I'm not convinced that init_builtin() or load_dynamic() are commonly used. These functions are more like internal functions used by the imp.load_module() function.

I'm not sure neither how find_module() was used with load_module() to load modules. Is the use case to specify a search path which is not in sys.path? Why not putting the path in sys.path and simply use importlib.import_module()?

I still consider that imp.load_source() is different and remains a relevant use case on its own. So I proposed PR #105978 to just document a recipe to replace this one.

vstinner added a commit that referenced this issue Jun 21, 2023
Explain how to port removed imp.load_source() to importlib in What's
New in Python 3.12.
bentasker pushed a commit to bentasker/cpython that referenced this issue Jun 23, 2023
Explain how to port removed imp.load_source() to importlib in What's
New in Python 3.12.
@brettcannon
Copy link
Member

I'm not sure neither how find_module() was used with load_module() to load modules. Is the use case to specify a search path which is not in sys.path?

I think so.

Why not putting the path in sys.path and simply use importlib.import_module()?

Worried they would forget to clean up sys.path after importing the one file they wanted?

miss-islington pushed a commit to miss-islington/cpython that referenced this issue Jun 25, 2023
Explain how to port removed imp.load_source() to importlib in What's
New in Python 3.12.
(cherry picked from commit 18a7c86)

Co-authored-by: Victor Stinner <vstinner@python.org>
vstinner added a commit that referenced this issue Jun 25, 2023
…106083)

gh-104212: Explain how to port imp.load_source() (GH-105978)

Explain how to port removed imp.load_source() to importlib in What's
New in Python 3.12.
(cherry picked from commit 18a7c86)

Co-authored-by: Victor Stinner <vstinner@python.org>
@vstinner
Copy link
Member

add it back to the docs, with explanations on how to migrate it to importlib
just document the migration in the release notes, in the section "Porting to Python 3.12"

That's now done in What's New in Python 3.12, but under the "Removed" section, where removals are mentioned. I dislike documenting the same removal at two places.

https://docs.python.org/dev/whatsnew/3.12.html#removed

I just wrote a 3.12 backport for my second doc change: PR #106083. It'ss going to be merged soon.

I added a recipe to replace imp.load_source(). I consider that this documentation issue is now solved and I close the issue.

If someone wants a more complete explanation for a specific removed imp function, please open a new issue.

Thanks @alexprengere for the bug report, thanks @brettcannon and @arhadthedev for the reviews.

melissalinkert added a commit to melissalinkert/ome-model that referenced this issue Mar 6, 2024
Ansuel added a commit to Ansuel/nmap that referenced this issue Apr 28, 2024
Python 3.12 deprecated and removed the already deprecated and not
documented imp library (the load_source function was never documented)

Replace this with a modern alternative suggested by
python/cpython#104212.

Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
Ansuel added a commit to Ansuel/nmap that referenced this issue Apr 28, 2024
Python 3.12 deprecated and removed the already deprecated and not
documented imp library (the load_source function was never documented)

Replace this with a modern alternative suggested by
python/cpython#104212.

Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
Ansuel added a commit to Ansuel/nmap that referenced this issue Apr 28, 2024
Python 3.12 deprecated and removed the already deprecated and not
documented imp library (the load_source function was never documented)

Replace this with a modern alternative suggested by
python/cpython#104212.

Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
Ansuel added a commit to Ansuel/nmap that referenced this issue Apr 28, 2024
Python 3.12 deprecated and removed the already deprecated and not
documented imp library (the load_source function was never documented)

Replace this with a modern alternative suggested by
python/cpython#104212.

Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
Ansuel added a commit to Ansuel/nmap that referenced this issue Apr 28, 2024
Python 3.12 deprecated and removed the already deprecated and not
documented imp library (the load_source function was never documented)

Replace this with a modern alternative suggested by
python/cpython#104212.

Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
Ansuel added a commit to Ansuel/nmap that referenced this issue Apr 28, 2024
Python 3.12 deprecated and removed the already deprecated and not
documented imp library (the load_source function was never documented)

Replace this with a modern alternative suggested by
python/cpython#104212.

Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
Ansuel added a commit to Ansuel/nmap that referenced this issue Apr 28, 2024
Python 3.12 deprecated and removed the already deprecated and not
documented imp library (the load_source function was never documented)

Replace this with a modern alternative suggested by
python/cpython#104212.

Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.11 only security fixes 3.12 bugs and security fixes docs Documentation in the Doc dir
Projects
None yet
Development

No branches or pull requests

7 participants