Issue14551
This issue tracker has been migrated to GitHub,
and is currently read-only.
For more information,
see the GitHub FAQs in the Python's Developer Guide.
Created on 2012-04-11 18:28 by r.david.murray, last changed 2022-04-11 14:57 by admin. This issue is now closed.
Messages (12) | |||
---|---|---|---|
msg158066 - (view) | Author: R. David Murray (r.david.murray) * ![]() |
Date: 2012-04-11 18:28 | |
This was removed in 2cf7bb2bbfb8 along with a bunch of other functions. Yet in issue 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. |
|||
msg158072 - (view) | Author: Antoine Pitrou (pitrou) * ![]() |
Date: 2012-04-11 20:36 | |
Hmm, yes, I don't remember exactly why, but it seems they were deprecated ("obsolete"), so it sounds reasonable to un-document them. |
|||
msg158074 - (view) | Author: R. David Murray (r.david.murray) * ![]() |
Date: 2012-04-11 20:56 | |
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 :) |
|||
msg158086 - (view) | Author: Brett Cannon (brett.cannon) * ![]() |
Date: 2012-04-11 22:44 | |
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. |
|||
msg158473 - (view) | Author: Brett Cannon (brett.cannon) * ![]() |
Date: 2012-04-16 15:45 | |
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. |
|||
msg158478 - (view) | Author: R. David Murray (r.david.murray) * ![]() |
Date: 2012-04-16 16:07 | |
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. |
|||
msg158480 - (view) | Author: Antoine Pitrou (pitrou) * ![]() |
Date: 2012-04-16 16:11 | |
> 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. |
|||
msg158488 - (view) | Author: Brett Cannon (brett.cannon) * ![]() |
Date: 2012-04-16 17:46 | |
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. |
|||
msg158490 - (view) | Author: R. David Murray (r.david.murray) * ![]() |
Date: 2012-04-16 18:10 | |
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. |
|||
msg158498 - (view) | Author: Brett Cannon (brett.cannon) * ![]() |
Date: 2012-04-16 21:13 | |
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. |
|||
msg158541 - (view) | Author: Antoine Pitrou (pitrou) * ![]() |
Date: 2012-04-17 10:51 | |
> 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. |
|||
msg158554 - (view) | Author: Brett Cannon (brett.cannon) * ![]() |
Date: 2012-04-17 16:07 | |
On Tue, Apr 17, 2012 at 06:51, Antoine Pitrou <report@bugs.python.org>wrote: > > Antoine Pitrou <pitrou@free.fr> 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. |
History | |||
---|---|---|---|
Date | User | Action | Args |
2022-04-11 14:57:29 | admin | set | github: 58756 |
2017-02-21 08:07:10 | THRlWiTi | set | nosy:
+ THRlWiTi |
2012-04-17 16:07:14 | brett.cannon | set | messages: + msg158554 |
2012-04-17 10:51:55 | pitrou | set | messages: + msg158541 |
2012-04-16 21:13:44 | brett.cannon | set | messages: + msg158498 |
2012-04-16 18:10:48 | r.david.murray | set | messages: + msg158490 |
2012-04-16 17:46:43 | brett.cannon | set | messages: + msg158488 |
2012-04-16 16:11:09 | pitrou | set | messages: + msg158480 |
2012-04-16 16:07:52 | r.david.murray | set | messages: + msg158478 |
2012-04-16 15:45:27 | brett.cannon | set | messages: + msg158473 |
2012-04-12 02:46:11 | eric.snow | set | nosy:
+ eric.snow |
2012-04-11 22:44:29 | brett.cannon | set | messages: + msg158086 |
2012-04-11 20:56:32 | r.david.murray | set | status: open -> closed resolution: not a bug messages: + msg158074 stage: resolved |
2012-04-11 20:36:43 | pitrou | set | messages: + msg158072 |
2012-04-11 18:28:30 | r.david.murray | create |