bpo-42403: Use @staticmethod in importlib

[vstinner]

Use @staticmethod on methods using @classmethod but don't use their
cls parameter, of the following classes:

• BuiltinImporter
• FrozenImporter
• WindowsRegistryFinder
• PathFinder

https://bugs.python.org/issue42403

[tiran]

What's the point? Are there any benefits in using staticmethod? I personally consider staticmethod a misfeature that should be avoided.

[vstinner]

What's the point? Are there any benefits in using staticmethod? I personally consider staticmethod a misfeature that should be avoided.

See https://julien.danjou.info/guide-python-static-class-abstract-methods/

I like @staticmethod to show that the method is not a method but a function put into the namespace of a class, to show that it does not use the class state (but can use external variables, like modules and the module global variables).

[vstinner]

Hum, test_importlib fails with this change. Example:

======================================================================

ERROR: test_for_top_level (test.test_importlib.test_metadata_api.APITests)

----------------------------------------------------------------------

Traceback (most recent call last):

  File "/home/travis/build/python/cpython/Lib/test/test_importlib/test_metadata_api.py", line 38, in test_for_top_level

    distribution('egginfo-pkg').read_text('top_level.txt').strip(),

AttributeError: 'NoneType' object has no attribute 'strip'

[vstinner]

Hum, test_importlib fails with this change.

It was a stupid mistake in my PR. It's now fixed ;-)

[JulienPalard]

I agree that using staticmethod+hardcoding the class name is a bad idea, but it's not the case here: semantically those are staticmethods, not classmethods as they don't need to know in which class they are. Marking them as so is an information to the reader.

[vstinner]

To be clear, this change doesn't prevent subclasses to access self or cls (if using @classmethod) in overriden methods.

[vstinner]

Marking them as so is an information to the reader.

Yeah, that's the main purpose of this change.

[vstinner]

PR rebased after I merged my other importlib PR #23397.

[vstinner]

@tiran: So are you ok with this change?

[tiran]

@tiran: So are you ok with this change?

I don't have a hard opinion here. If you like to change it, go ahead.

I personally don't like staticmethod because I don't see a valid use case for it. I see them as sign of bad design. Either a method is bound to an instance, bound to a class, or it should be a function in the first place.

[vstinner]

I personally don't like staticmethod because I don't see a valid use case for it. I see them as sign of bad design. Either a method is bound to an instance, bound to a class, or it should be a function in the first place.

The importer API requires create_module(sep) to be a method, to be able to call my_impoter.create_module(sep). See importlib.abc.Loader ABC. The implementation is free to use a static method (no extra parameter), a class method (cls) or a regular method (self).

• ExtensionFileLoader.create_module() uses self
• BuiltinImporter.create_module() doesn't use self nor self
• LazyLoader.create_module() uses self

I'm a little bit confused between "loader" and "importer", but it doesn't matter much to explain my point ;-)

Marking a method with @staticmethod or @classmethod doesn't prevent to override the method in a subclass with any kind of method (static method, class method or regular method). Calling parent super().create_module(spef) method also works in all cases.