Skip to content

Conversation

@dr-carlos
Copy link
Contributor

@dr-carlos dr-carlos commented Dec 5, 2025

  • Clarified in the glossary that an annotate function can be any callable (as is already the case).
  • Improved error messages if call_annotate_function() tries to run with fake globals but can't find the annotate function's __code__ attribute.
  • Added defaults if other non-required attributes cannot be found for fake globals, e.g. if the annotate function doesn't provide __defaults__, that's fine, just assume it's None.
    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 to builtins.__dict__).
  • Added tests for the current support, added support, and error messages associated with non-function callables as annotate functions..
  • Added a section to the annotationlib documentation 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.

📚 Documentation preview 📚: https://cpython-previews--142327.org.readthedocs.build/

Copy link
Member

@sobolevn sobolevn left a comment

Choose a reason for hiding this comment

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

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.

Copy link
Member

@picnixz picnixz left a comment

Choose a reason for hiding this comment

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

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".

class Annotate:
called_formats = []
def __call__(self, format=None, *, _self=None):
Copy link
Member

Choose a reason for hiding this comment

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

Is this the signature we want to force?

Copy link
Contributor Author

@dr-carlos dr-carlos Dec 7, 2025

Choose a reason for hiding this comment

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

Well, we're not really forcing it, but you're right that it is a somewhat awkward suggestion. I'll have a think about alternatives.

We could also move the logic in __call__ to a separate method and then do the self and _self handling within __call__ before passing to the method.

@dr-carlos
Copy link
Contributor Author

dr-carlos commented Dec 7, 2025

Seems like there's some consensus that it's not worth supporting non-functions any more than they currently are. I still think it's worth supporting them (see #141388), but if it's left as a bit of hassle, then that's alright since it's an advanced feature.
So, I've reduced this to just a documentation and test patch.

I'll update the NEWS entry and add the missing attributes to docs soon when I have some time :)

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.

3 participants