gh-141388: Improve support for non-function callables as annotate functions #142327
Conversation
… __defaults__, and __kwdefaults__
sobolevn
left a comment
sobolevn
left a comment
There was a problem hiding this comment.
I am not quite convinced that this should be done. __annotate__ is already complex enough.
In the issue itself there were no real use-cases that can support the idea.
In my opinion, it is fine to have __annotate__ magic function as function only.
There was a problem hiding this comment.
I really prefer this version where we leave most of the work to the user because it's an advanced feature that they need to respect. But I also don't know whether it's worth it. It kinda slows down annotate functions for everyone. Maybe just documenting what needs to be present is sufficient but we maybe just say "Your callable should behave like a function, with the same dunders".
| try: | ||
| annotate_code = annotate.__code__ | ||
| except AttributeError: | ||
| raise AttributeError( | ||
| "annotate function requires __code__ attribute", | ||
| name="__code__", | ||
| obj=annotate | ||
| ) |
There was a problem hiding this comment.
I think we can just propagate the AttributeError without this wrapping.
| annotate, owner, is_class, globals, allow_evaluation=True | ||
| annotate, owner, is_class, globals, annotate_globals, allow_evaluation=True | ||
| ) | ||
| try: |
| ) | ||
| closure, cell_dict = _build_closure( | ||
| annotate, owner, is_class, globals, allow_evaluation=False | ||
| annotate, owner, is_class, globals, annotate_globals, allow_evaluation=False |
There was a problem hiding this comment.
Wrap this under 80 chars (I think we can split after globals)
There was a problem hiding this comment.
Only use 2 blank lines to separate sections.
| This can then be called with: | ||
|
|
||
| .. code-block:: python |
There was a problem hiding this comment.
| .. code-block:: python | |
| .. code-block:: pycon |
| {'x': 'MyType'} | ||
There was a problem hiding this comment.
| {'x': 'MyType'} | |
| {'x': 'MyType'} | |
|
|
||
| .. code-block:: python | ||
| class Annotate: |
There was a problem hiding this comment.
Indent the entire code block by 3 spaces (and then 4 spaces)
| @property | ||
| def __defaults__(self): | ||
| return (None,) | ||
| @property | ||
| def __kwdefaults__(self): | ||
| return {"_self": self} | ||
| @property | ||
| def __code__(self): | ||
| return self.__call__.__code__ |
There was a problem hiding this comment.
| @property | |
| def __defaults__(self): | |
| return (None,) | |
| @property | |
| def __kwdefaults__(self): | |
| return {"_self": self} | |
| @property | |
| def __code__(self): | |
| return self.__call__.__code__ | |
| __defaults__ = (None,) | |
| __kwdefaults__ = property(lambda self: dict(_self=self)) | |
| __code__ = property(lambda self: self.__call__.__code__) |
| class Annotate: | ||
| called_formats = [] | ||
| def __call__(self, format=None, *, _self=None): |
There was a problem hiding this comment.
Is this the signature we want to force?
|
|
||
| .. code-block:: python | ||
| class Annotate: |
There was a problem hiding this comment.
I would rather have a protocol being explained, e.g.:
The annotate protocol is an object with the following proeprties:
- A callable ``__call__`` attribute with the following signature: ...
- Properties `__defaults__`, `__kwdefaults__` and `__code__` such that ...It's not clear what is required and what's not and what we can put inside. And then you can have an example.
3 participants
call_annotate_function()tries to run with fake globals but can't find the annotate function's__code__attribute.__defaults__, that's fine, just assume it'sNone.If required, I can remove some (or all) of these defaults, particularly
__globals__which adds an extra parameter to_build_closure(), or__builtins__which is semantically correct but a little more complicated than the rest (it would be much simpler to just fallback tobuiltins.__dict__).annotationlibdocumentation providing a simple recipe for a 'minimum viable implementation' of a custom callable class as annotate function. Happy to remove if this is unnecessary/too niche - I just thought it was the clearest way of demonstrating how to implement one of these, without just spelling out which dunders they need to implement.__annotate__Functions don't actually need to be functions #141388📚 Documentation preview 📚: https://cpython-previews--142327.org.readthedocs.build/