homepage

Various places list explicitly what abstractmethods are needed to make an ABC work in importlib.abc. Unfortunately when module_repr() came into existence it was made abstract but no other documentation was updated to point out this fact.

Barry, Eric: can you clarify why you made module_repr an abstractmethod and thus require its overloading? It seems like its default is fine and you should only need to overload it when you can say something better than the default.

On Apr 01, 2013, at 07:40 PM, Brett Cannon wrote:
>Barry, Eric: can you clarify why you made module_repr an abstractmethod and
>thus require its overloading?
Maybe Eric can, but I can't. ;) I honestly don't remember why we made it
abstract, except perhaps because we put it in the Loader class which already
had an abstract load_module() method.
>It seems like its default is fine and you should only need to overload it
>when you can say something better than the default.
Note that moduleobject.c can handle the case of a missing or uncallable
module_repr() attribute. Then it implements the defaults specified in PEP
420.

If Eric doesn't have anything to add then I would like to change importlib.abc.Loader.module_repr() to no longer be abstract and the default to be defined as ``return repr(module)``. Else it should be entirely optional and not have a default implementation, but having it be required as abstract doesn't seem quite right to me.

I was going to blame Barry, but I see he beat me to it :)
It looks like an oversight, and it shouldn't be abstract.

On Apr 01, 2013, at 09:04 PM, Brett Cannon wrote:
>If Eric doesn't have anything to add then I would like to change
>importlib.abc.Loader.module_repr() to no longer be abstract and the default
>to be defined as ``return repr(module)``. Else it should be entirely optional
>and not have a default implementation, but having it be required as abstract
>doesn't seem quite right to me.
I have no problem making it non-abstract, but I don't think you need to define
a default. The built-in module object already does the right thing by
default. Maybe that's why we (mistakenly?) made it abstract. We might have
hoped to document the API but not provide a default implementation, since if
module_repr() is not both defined and callable, moduleobject.c already does
the right thing.
But at least there are unit tests so it should be hard for you to break
things. :)

Totally optional and no default argument it is.

Fixed by http://hg.python.org/cpython/rev/8e733e30edf6 by raising NotImplementedError and relying on the fact that ModuleType.__repr__() swallows any exception.