`__annotate__` Functions don't actually need to be functions

[dr-carlos]

Bug report

Bug description:

According to the Python docs, annotate functions need to be actual functions. However, the C code doesn't enforce this, it enforces (using PyCallable_Check()) that the annotate 'function' is any callable:

cpython/Objects/funcobject.c

Lines 865 to 888 in 9cb8c52

	static int
	function___annotate___set_impl(PyFunctionObject *self, PyObject *value)
	/*[clinic end generated code: output=05b7dfc07ada66cd input=eb6225e358d97448]*/
	{
	if (value == NULL) {
	PyErr_SetString(PyExc_TypeError,
	"__annotate__ cannot be deleted");
	return -1;
	}
	if (Py_IsNone(value)) {
	Py_XSETREF(self->func_annotate, value);
	return 0;
	}
	else if (PyCallable_Check(value)) {
	Py_XSETREF(self->func_annotate, Py_XNewRef(value));
	Py_CLEAR(self->func_annotations);
	return 0;
	}
	else {
	PyErr_SetString(PyExc_TypeError,
	"__annotate__ must be callable or None");
	return -1;
	}
	}

This initially seems fine, but leads to some weird bugs.

class C:
  def __call__(self, format, /):
    if format > 2:
      raise NotImplementedError
    return {'x': int}

def f(x): ...
f.__annotate__ = C()

from annotationlib import get_annotations, Format
get_annotations(f, format=Format.STRING) # AttributeError: 'C' object has no attribute '__closure__' ...

And similar for get_annotations(f, format=Format.FORWARDREF), trying to find the annotation function's __builtins__, which only exists on functions (not arbitrary callables).

I'm happy to make a PR to replace the PyCallable_Check() with PyFunction_Check() in the relevant C code for setting __annotate__, but I guess this might be a breaking change? I'm not sure, so I thought I'd check first before making it. The other option would be checking for the presence of __closure__/__builtins__/etc. before accessing them in annotationlib.

CPython versions tested on:

CPython main branch

Operating systems tested on:

Linux

Linked PRs

• gh-141388: Fully support non-function callables as annotate functions #141449
• gh-141388: Improve docs/tests for non-function callables as annotate functions #142327

[johnslavik]

I guess this might be a breaking change

Who would be affected?

[JelleZijlstra]

Who would be affected?

People who upgraded to 3.14 and immediately did weird things with annotate functions, I suppose. I would assume that's not a large set of people but disallowing non-function callables would indeed technically be breaking.

Open to other opinions but I think we should try to support non-function callables. However, if the callable doesn't have attributes that we rely on, we can error out.

[dr-carlos]

Open to other opinions but I think we should try to support non-function callables. However, if the callable doesn't have attributes that we rely on, we can error out.

Sounds good! Only allowing functions is a bit restrictive, so I agree that it makes sense to provide best-effort support. I'll have a look at what changes we would need to support them and open a PR if it looks reasonable.

[dr-carlos]

@JelleZijlstra I've opened a draft PR to investigate how we might support various non-function callable types. Would you be able to take a look at some point and let me know if you think it's still worth pursuing, or if there might be a better way to implement this?

[picnixz]

I guess this might be a breaking change

Who would be affected?

Everyone who wants to use a class to have better encapsulation purposes. It allows to add a state on __annotate__ itself or to factor its implementation in methods (though private module-level functions are likely better most of the time but this is a matter of taste).

However such people would need to provide callables that have the desired properties.

[dr-carlos]

I was going to just improve docs and implement more informative errors for missing attributes on annotate functions. But I saw that a lot of the attributes didn't really need to be there (e.g. __kwdefaults__ or __closure__) for the fake globals machinery to work, so in #142327 I've added defaults for these.

If that's still too much complexity, these defaults can be removed - it just requires users to provide callables with a lot of boilerplate.

[dr-carlos]

@picnixz Moving the conversation back from #142327, currently we support:

• "Standard formats" (value, string, fwdref): any callable works, we call the thing, no trickery needed.
• Value with fake globals: specifically requires expected attributes of a function.

Value with fake globals is not necessary for a custom annotate function to implement (they could just implement string and fwdref outright), but the ForwardRef constructor is not documented, so we currently have no documented way to support ForwardRefs in custom annotate functions (since iirc, value with fake globals documentation doesn't really exist, but that's a different discussion).

Anyway, since this is still a very new feature - it might be simpler for now to disallow non-functions in general (since they only half-work), see how people write their annotate functions, and relax the restriction in the future if necessary?

What do others think?