Re: PEP 420 issue: extend_path

On Tue, May 8, 2012 at 3:09 PM, Eric V. Smith <[email protected]> wrote:
> On 05/07/2012 11:57 PM, Eric V. Smith wrote:
>
>> I think I'll modify the code in the pep-420 branch with find_package(),
>> keeping find_module() unmodified from the version in the 3.3 branch.
>> Assuming that works out, I'll modify the PEP and point it to this
>> discussion.
>
> I've checked the find_package() version in to the pep-420 branch. I'm
> still not wild about the name, but I think leaving find_module()
> unmodified is an improvement. After some discussion and a possible name
> change, I'll update PEP 420.

Yes, I'd forgotten that we need to check the rest of the path for
ordinary modules as well as self-contained packages after finding the
initial directory, so my proposed adjustment doesn't actually save any
complexity and my proposed method name is simply wrong.

What we're really talking about now is a wholesale replacement for
find_module with the new semantics (i.e. allowing a string to be
returned for namespace package paths), rather than PEP 382's extra
processing step (where find_module would still be called, even if
find_package_portion was defined).

So, let's call the replacement finder method "find_loader", since
that's really what it's for (find_module was always a bit of a
misnomer, since the method returns a loader object, not a module).

This is where I start to have sympathy for Antoine's point of view:
we're overloading the meaning of returning a string from find_loader()
to say two things:
1. Here's my sys.path entry
2. Please continue scanning sys.path for additional entries

Consider the following possible signature that avoids that overloading
by passing in a separate callback for the "found a directory" aspect:

    def find_loader(fullname, dir_found=None)
        # dir_found is a callback that gets invoked if a matching
directory is found
        if dir_found is None:
            def dir_found(dirpath):
                msg = "Not importing directory {}: missing __init__"
                _warnings.warn(msg.format(dirpath), ImportWarning)
        # As per existing find_module, but calls dir_found(base_path) instead
        # emitting ImportWarning directly


This would then be used roughly as follows:
    package_path = []
    for importer in _iter_importers(path):
        try:
            find_loader = importer.find_loader
        except AttributeError:
            # Backwards compatibility with the original PEP 302 finder API
            loader = importer.find_module(fullname)
        else:
            loader = importer.find_loader(fullname, package_path.append)
        if loader is not None:
            return loader
    if package_path:
        return NamespaceLoader(package_path)


And the delegation from find_module would look like:

    def find_module(self, fullname):
        """Try to find a loader for the specified module."""
        return self.find_loader(fullname)

Advantages of this approach:
- cleanly separates "here's my sys.path entry" (dir_found callback)
and "please continue scanning sys.path" (None return value)
- obvious and accurate name for the new method (i.e. "find_loader")
- provides additional flexibility to import hook consumers
- trivial to reproduce old API behaviour with the new API if desired

Cheers,
Nick.

-- 
Nick Coghlan   |   [email protected]   |   Brisbane, Australia