-
Couldn't load subscription status.
- Fork 2.7k
Refactor func_metadata() implementation
#1496
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
base: main
Are you sure you want to change the base?
Conversation
| "typing-extensions>=4.9.0", | ||
| "typing-inspection>=0.4.1", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR keeps the support for Pydantic 2.11.
@Viicos is maintaining Pydantic, so he knows stuff.
5b71783 to
5dff9fa
Compare
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we already have a unit test that covers this? otherwise would be good to add one
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
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