Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP gh-103741 - PEP 713 - Callable modules #103742

Closed
wants to merge 4 commits into from

Conversation

amyreese
Copy link
Contributor

@amyreese amyreese commented Apr 24, 2023

Draft implementation of PEP 713:

  • Implement ability to call modules with __call__ property.
  • Unit tests covering behavior for callable and non-callable modules
  • Documentation changes
  • News entry

Adds a method wrapper for module objects that makes them callable.
When called, checks for a __call__ object on the module and calls
that instead, otherwise raises type error declaring the module
is not callable.

Adds a check in module_getattro to prevent returning the method
wrapper when looking for __call__, to prevent circular lookups
when calling the module and checking for __call__ set by module
authors.

Adds PyModule_Callable helper function to determine if a module
is callable, by looking for a __call__ property and calling the
normal PyCallable_Check on that object, if it exists.

Updates PyCallable_Check to use the new PyModule_Callable helper
when checking module objects, to correctly look for a callable
__call__ property rather than the normal tp_call attribute on the
module's type struct.

Tested invariants:

  • For modules without __call__ (eg, stdlib):

    • callable(mod) -> False
    • mod.__call__ -> AttributeError
    • mod() -> TypeError
  • For module with __call__ or __getattr__ returning __call__ object

    • callable(mod) -> callable(mod.__call__)
    • mod.__call__ -> real object
    • mod() -> mod.__call__()

Setting (or unsetting) __call__ on existing module objects makes
them callable (or uncallable) accordingly, without changing the type
or class of the module object.

When making calls to core types, if no `tp_call` value is found (the
type isn't default callable) and the type in question is a `ModuleType`,
then `_PyObject_MakeTpCall` will use getattr to check for a `__call__`
attribute on the module object, and attempt to call that instead.

When checking if an object is callable, the initial `tp_call` lookup
doesn't find a value, and the object type is a module, a similar getattr
for `__call__` is made, and a callable check is made against that value
instead.

This is implemented this way, rather than by adding a `tp_call` method
directly to the base module type, as not all modules will be callable,
and the existence of a method wrapper makes it difficult to determine
if the module in question is actually callable, or just has a method
wrapper. This eliminates the need for isinstance checks and special
error handling in said method wrapper to make non-callable modules
continue to appear as non-callable objects.
Copy link
Member

@Fidget-Spinner Fidget-Spinner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the effort in writing this PEP. I have some minor comments below.

Objects/call.c Outdated Show resolved Hide resolved
Objects/object.c Outdated Show resolved Hide resolved
@Fidget-Spinner
Copy link
Member

Adding the do not merge label, will remove when the PEP is approved.

@amyreese
Copy link
Contributor Author

Also, I'm unsure why CI is failing during build; I can build this PR on my Mac Mini and my Debian box without any errors.

@Fidget-Spinner
Copy link
Member

CI is failing because of the test suite. run ./python -m test -j<number of cores>. It seems that test_typing is failing. So you might want to just run ./python -m test test_typing.

Objects/object.c Outdated Show resolved Hide resolved
Lib/test/test_call.py Outdated Show resolved Hide resolved
ambv and others added 2 commits April 25, 2023 02:32
Adds a method wrapper for module objects that makes them callable.
When called, checks for a __call__ object on the module and calls
that instead, otherwise raises type error declaring the module
is not callable.

Adds a check in module_getattro to prevent returning the method
wrapper when looking for __call__, to prevent circular lookups
when calling the module and checking for __call__ set by module
authors.

Adds PyModule_Callable helper function to determine if a module
is callable, by looking for a __call__ property and calling the
normal PyCallable_Check on that object, if it exists.

Updates PyCallable_Check to use the new PyModule_Callable helper
when checking module objects, to correctly look for a callable
__call__ property rather than the normal tp_call attribute on the
module's type struct.

Tested invariants:

- For modules without __call__ (eg, stdlib):
  - callable(mod) -> False
  - mod.__call__ -> AttributeError
  - mod() -> TypeError

- For module with __call__ or __getattr__ returning __call__ object
  - callable(mod) -> callable(mod.__call__)
  - mod.__call__ -> real object
  - mod() -> mod.__call__()

Setting (or unsetting) __call__ on existing module objects makes
them callable (or uncallable) accordingly, without changing the type
or class of the module object.

Co-authored-by: Hood Chatham <roberthoodchatham@gmail.com>
@amyreese
Copy link
Contributor Author

Replaced the first draft with an implementation based on setting tp_call, as suggested by @Fidget-Spinner, with special cases in callable and module_getattro to correctly expose __call__ only when actually set in the module's __dict__ or __getattr__, with the better error message format when calling an uncallable module, as suggested by @JelleZijlstra.

Copy link
Contributor

@erlend-aasland erlend-aasland left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You also need to add NEWS and What's New entries.

del sys.__call__

def test_callable_modules(self):
from . import callable_module_a, callable_module_b, callable_module_c
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer inlining these instead of polluting the Lib/test namespace with the added files. Alternatively, put them in a subdirectory.

@@ -1668,6 +1668,9 @@ PyCallable_Check(PyObject *x)
{
if (x == NULL)
return 0;
if (PyModule_CheckExact(x)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This excludes subclasses of ModuleType.

@@ -34,6 +34,7 @@ PyAPI_FUNC(int) _PyModuleSpec_IsInitializing(PyObject *);
#endif
PyAPI_FUNC(PyModuleDef*) PyModule_GetDef(PyObject*);
PyAPI_FUNC(void*) PyModule_GetState(PyObject*);
PyAPI_FUNC(int) PyModule_Callable(PyObject *);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be in Include/cpython/*.h or in an #ifndef Py_LIMITED_API block.
Or, if you want to add it to the limited API, see docs in the devguide.

@sunmy2019
Copy link
Member

sunmy2019 commented May 17, 2023

It looks like it did not make it to 3.12.

@erlend-aasland
Copy link
Contributor

It looks like it did not make it to 3.12.

The SC has not even decided about the PEP yet (see python/steering-council#191).

@AlexWaygood
Copy link
Member

AlexWaygood commented Sep 22, 2023

Closing for now, since the PEP was rejected by the Steering Council. But thank you for writing PEP-713 -- it was a really interesting discussion, I thought!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants