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

imp.load_source docs removed from python3 docs...is this correct? #58756

Closed
bitdancer opened this issue Apr 11, 2012 · 13 comments
Closed

imp.load_source docs removed from python3 docs...is this correct? #58756

bitdancer opened this issue Apr 11, 2012 · 13 comments
Labels
type-bug An unexpected behavior, bug, or error

Comments

@bitdancer
Copy link
Member

BPO 14551
Nosy @brettcannon, @pitrou, @bitdancer, @ericsnowcurrently

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:

assignee = None
closed_at = <Date 2012-04-11.20:56:32.705>
created_at = <Date 2012-04-11.18:28:30.737>
labels = ['type-bug', 'invalid']
title = 'imp.load_source docs removed from python3 docs...is this correct?'
updated_at = <Date 2017-02-21.08:07:10.018>
user = 'https://github.com/bitdancer'

bugs.python.org fields:

activity = <Date 2017-02-21.08:07:10.018>
actor = 'THRlWiTi'
assignee = 'none'
closed = True
closed_date = <Date 2012-04-11.20:56:32.705>
closer = 'r.david.murray'
components = []
creation = <Date 2012-04-11.18:28:30.737>
creator = 'r.david.murray'
dependencies = []
files = []
hgrepos = []
issue_num = 14551
keywords = []
message_count = 12.0
messages = ['158066', '158072', '158074', '158086', '158473', '158478', '158480', '158488', '158490', '158498', '158541', '158554']
nosy_count = 5.0
nosy_names = ['brett.cannon', 'pitrou', 'r.david.murray', 'THRlWiTi', 'eric.snow']
pr_nums = []
priority = 'normal'
resolution = 'not a bug'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue14551'
versions = ['Python 3.2', 'Python 3.3']

@bitdancer
Copy link
Member Author

This was removed in 2cf7bb2bbfb8 along with a bunch of other functions. Yet in bpo-13959 Brett mentions needing to implement it. I don't see any replacement for its functionality in importlib, so I would think it would be a function we would want to keep. It certainly still exists, and I'm sure many people are using it.

@bitdancer bitdancer added the type-bug An unexpected behavior, bug, or error label Apr 11, 2012
@pitrou
Copy link
Member

pitrou commented Apr 11, 2012

Hmm, yes, I don't remember exactly why, but it seems they were deprecated ("obsolete"), so it sounds reasonable to un-document them.

@bitdancer
Copy link
Member Author

OK, the text at the start of the section, that I didn't notice in the 2.7 docs, says they are obsolete and replaced by find_module and import_module. But load_source is much more convenient, so I for one am not going to remove my use of it in the tests I just checked in. Someone who wants to actually remove load_source can fix them :)

@brettcannon
Copy link
Member

Once importlib bootstrapping lands and I expose the rest of the API you can replace imp.load_source() w/ importlib.SourceFileLoader(name, path).load_module(name) and get the same result.

@brettcannon
Copy link
Member

David, did you find load_source() convenient because you could specify the file to use for the module's source? Did you actually like the file object argument? Just trying to gauge if some new API is needed on a loader of if the one-liner I proposed is good enough.

@bitdancer
Copy link
Member Author

The one-liner is "good enough", but...

The use case is indeed loading a module from an arbitrary file without having to worry about sys.path, etc. For that load_source is intuitive. The one-liner is an adequate substitute, but feels like a step backward for this particular use case. That is, load_source is a *convenience* function, from my POV :)

I did not use the 'file' argument. If I were dealing with an already open file it would seem more natural to use imp.load_module.

@pitrou
Copy link
Member

pitrou commented Apr 16, 2012

The one-liner is an adequate substitute, but feels like a step
backward for this particular use case. That is, load_source is a
*convenience* function, from my POV :)

Agreed with David.

@brettcannon
Copy link
Member

To help refine this, so you would expect all of the usual import stuff (e.g. sys.modules use, generating bytecode, etc.), you just want a short-circuit to the loading when you happen to already know the name and desired file path?

Basically I want to kill off the file argument to imp.load_source() since it buys you nothing as import is no longer structured to work off of already opened files, and especially files opened to return text instead of bytes (the only reason load_*() even takes an open file is because it directly exposes stuff part way through import.c's import process).

And as for having an already opened file, don't count on load_module() sticking around since find_module() is going to go since, once again, it is only structured the way it is with its poor API because of how import.c's C code was structured.

Thinking about it a little, would importlib.util.SourceFileLoader(name, path).load_source() be an okay API for you? Or do you really want a function instead of a convenience method on a loader instance? I basically want to gut imp down to only stuff that is required for backwards-compatibility for for really low-level stuff that normal people never touch and have common stuff like load_source() be in importlib somewhere.

@bitdancer
Copy link
Member Author

Well, if you want backward compatibility, you pretty much have to keep it as is, don't you?

This is the only time I've used load_source, and it was used because we didn't want to bother mucking with the path but did want to load the module whose file system location we knew.

SourceFileLoader(name, path).load_source() is OK, but it does not qualify as an obvious API for loading a module not on the path whose file system path one knows. I'm OK with there not being an obvious API for that as long as there *is* an API for it.

To be clear: importlib.import_module(name) lets you load a module that *is* on the path, so it seems logical that there would be a corresponding function (load_file?) that would take a complete path and thereby allow you to load a module *not* on sys.path. But there is by no means a *requirement* for such a function in my mind, as long as it can be done somehow.

@brettcannon
Copy link
Member

Well, I want backwards-compatibility *now*, not forever.

And no, it is not an obvious API as you are asking for what loaders are supposed to do; load a module, which is why the one-liner I gave you works today. Finder simply find a loader that can load something that they are asked to discover. Loader then load something that they are told to load, whether it was found through a finder for sys.path or explicitly pointed at something.

Another option is to have SourceFileLoader.load_module() take no argument since its constructor already has everything needed to load a module passed in which simplifies the API a little bit. But that would be a non-standard broadening of the loader API and I don't know if people will like that.

@pitrou
Copy link
Member

pitrou commented Apr 17, 2012

Well, I want backwards-compatibility *now*, not forever.

I don't think changing a function signature in an incompatible way is
generally acceptable. You might make one of the arguments optional,
though (but keeping the current semantics when the argument *is*
passed). If it's not possible, you can add another function with the
intended behaviour.

The importlib bootstrapping has already had some (unavoidable)
disruptive consequences. Let's keep them to a minimum. People *rely* on
our APIs, even the less popular ones.

@brettcannon
Copy link
Member

On Tue, Apr 17, 2012 at 06:51, Antoine Pitrou <[email protected]>wrote:

Antoine Pitrou <[email protected]> added the comment:

> Well, I want backwards-compatibility *now*, not forever.

I don't think changing a function signature in an incompatible way is
generally acceptable.

I don't think it is either.

You might make one of the arguments optional,
though (but keeping the current semantics when the argument *is*
passed). If it's not possible, you can add another function with the
intended behaviour.

Right, which is why I'm thinking that I could make the module name argument
optional for load_module() to avoid repeating yourself since that
information is passed to the constructor.

The importlib bootstrapping has already had some (unavoidable)
disruptive consequences. Let's keep them to a minimum. People *rely* on
our APIs, even the less popular ones.

Which is unfortunate when the API is bad. Anyway, the deprecation can be a
long one, but I don't want people having to look in two places for
import-related stuff like urllib/urllib2 caused for URLs.

@vstinner
Copy link
Member

I proposed #105755 to add importlib.util.load_source() function.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

No branches or pull requests

4 participants