Refactor func_metadata() implementation
#1496
Conversation
| "typing-extensions>=4.9.0", | ||
| "typing-inspection>=0.4.1", |
There was a problem hiding this comment.
typing-extensions is already used in the project, but not declared as an explicit dependency. 4.9.0 is the oldest version with (proper) support for typing_extensions.deprecated.
typing-inspection is already a Pydantic dependency, so nothing new added here.
| def _get_typed_signature(call: Callable[..., Any]) -> inspect.Signature: | ||
| """Get function signature while evaluating forward references""" | ||
| signature = inspect.signature(call) | ||
| globalns = getattr(call, "__globals__", {}) | ||
| typed_params = [ | ||
| inspect.Parameter( | ||
| name=param.name, | ||
| kind=param.kind, | ||
| default=param.default, | ||
| annotation=_get_typed_annotation(param.annotation, globalns), | ||
| ) | ||
| for param in signature.parameters.values() | ||
| ] | ||
| typed_return = _get_typed_annotation(signature.return_annotation, globalns) | ||
| typed_signature = inspect.Signature(typed_params, return_annotation=typed_return) | ||
| return typed_signature |
There was a problem hiding this comment.
Removed, we can just use inspect.get_annotations() with eval_str=True.
It also enables more use cases to be supported, e.g.:
from __future__ import annotations
def func[T](a: T): ...
from_metadata(func) # Used to raiseLet me know if I should add a test for this.
Viicos
force-pushed
the
refactor-func-metadata
branch
2 times, most recently
from
5dff9fa to
cf3ae5c
Compare
| if return_type_expr is UNKNOWN and structured_output is True: | ||
| # `return_type_expr` is `UNKNOWN` when a bare type qualifier is used (unlikely to happen as a return annotation | ||
| # because it doesn't make any sense, but technically possible). | ||
| raise InvalidSignature(f"Function {func.__name__}: return annotation required for structured output") |
There was a problem hiding this comment.
do we already have a unit test that covers this? otherwise would be good to add one
| # Reconstruct the original annotation, by preserving the remaining metadata, | ||
| # i.e. from `Annotated[CallToolResult, ReturnType, Gt(1)]` to | ||
| # `Annotated[ReturnType, Gt(1)]`: | ||
| original_annotation = Annotated[(return_type_expr, *inspected_return_ann.metadata[1:])] |
There was a problem hiding this comment.
if I'm reading this right, does that mean the current code on main would strip the extra metadata and cause different Pydantic models to be created? if so a unit test for this seems like a good idea
There was a problem hiding this comment.
The code on main behaves as expected (in most cases, see here where it doesn't on main and on this PR), because it is constructing a FieldInfo instance from the annotation, and passing it again as Annotated[ann, field_info_instance], which is an undocumented pattern although kind of works.
I'll add tests related to this.
felixweinberger
added
enhancement
needs more work
Viicos
force-pushed
the
refactor-func-metadata
branch
from
961b3cc to
2b607bc
Compare
felixweinberger
added
enhancement
maxisbey
force-pushed
the
refactor-func-metadata
branch
from
2b607bc to
1e0e002
Compare
|
Thanks @maxisbey (sorry I'm out for some days couldn't rebase). |
4 participants
Motivation and Context
This is a first step in adding support for Pydantic 2.12 and Python 3.14. It avoids relying on patterns that we discourage using in Pydantic (mostly around the
FieldInfoclass), and removes usage of private utilities.How Has This Been Tested?
Breaking Changes
Types of changes
Checklist
Additional context