[BUG] Tiny regression introduced by changing methods signatures of `_EditableFinder`

[mtreglia-gpsw]

setuptools version

setuptools>=70.0.0

Python version

Python3.10

OS

macOS

Additional environment information

Using a custom implementation for live reloading python code.

Description

Upgrading to setuptools>=70.0.0 I notice some failure in some simple use case due to the change of signature of the _EditableFinder used when a python module is installed in editable mode.

The change was introduced in this commit-19b63d1b and it alter the standard signature of the classmethod find_spec of a PathFinder.

In fixing the PathEntryFinder a change of the signature was introduced for argument _path and _target:

# From 
def find_spec(fullname, path=None, target=None):
    ...
# To 
def find_spec(cls, fullname: str, _path=None, _target=None):
    ...

Expected behavior

The _EditableFinder use the standard PathFinder naming convention for the methods's arguments.

How to Reproduce

1. Install any local python package in editable mode
2. Add the import of the recently installed package in the script's main
3. Run the script.

import sys
import functools

def install():
    """Install the patch to the sys.meta_path finders classes.
    This function replaces the `find_spec` function of each finder to
    ensure that any ModuleSpecs will use the `ModuleLoader` internally`
    """

    # Wrapper for find_spec
    def wrap_find_spec(find_spec):
        @functools.wraps(find_spec)
        def wrapper(fullname, path, target=None):
            spec = find_spec(fullname=fullname, path=path, target=target)
            if spec is not None:
                # Do other stuff with the spec.loader
                pass
            return spec

        return wrapper

    # Wrap system meta path finders
    for finder in sys.meta_path:
        # Override the functions we care about
        if hasattr(finder, "find_spec"):
            setattr(finder, "find_spec", wrap_find_spec(finder.find_spec))

if __name__ == "__main__":
    install()
    # Now import a python package installed in editable mode
    # import <editable-package>

Output

❯ python main.py
Traceback (most recent call last):
  File "/Users/mtreglia/setuptools-repro/main.py", line 34, in <module>
    import sandbox
  File "<frozen importlib._bootstrap>", line 1027, in _find_and_load
  File "<frozen importlib._bootstrap>", line 1002, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 945, in _find_spec
  File "/Users/mtreglia/setuptools-repro/main.py", line 15, in wrapper
    spec = find_spec(fullname=fullname, path=path, target=target)
TypeError: _EditableFinder.find_spec() got an unexpected keyword argument 'path'

[abravalheri]

Hi @mtreglia-gpsw, my currently understanding is that Python docs nudge towards what we would call nowadays "protocol".

When discussing protocols in this context, the feedback that I once got was the following:

Protocols should usually have positional-only parameters, because otherwise implementations need to use the exact same parameter names. Use positional-or-keyword parameters only if consumers of the protocol pass keyword arguments.

So I think it makes more sense for "client code" to use positional-style calling conventions. Otherwise it will always be at the risk of exceptions. For example, in the code snippet that you gave, I think that at least relying on fullname= keyword style invocation is problematic.

That said, I am happy to consider a PR (we probably need to add the specific type: ignore annotation though). Would you like to submit one?

[mtreglia-gpsw]

Hi @abravalheri , thanks for the quickly reply.

I see, thanks also for sharing the conversation link, indeed the protocol in _typeshed.importlib seems to be the way 👍 .

I'w be glad to propose a PR in the following days.

[mtreglia-gpsw]

Hi @abravalheri, I made the PR, could you have a look please ? https://github.com/pypa/setuptools/pull/4470 Thanks in advance.

[abravalheri]

Thank you very much for the PR.

This will be available on the next release of setuptools.