Skip to content
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

Enhancement: ParsedSignature to enable TypeVar expansion using signature namespace #3206

Open
harryle95 opened this issue Mar 15, 2024 · 7 comments
Labels
Enhancement This is a new feature or request

Comments

@harryle95
Copy link
Contributor

harryle95 commented Mar 15, 2024

Summary

ParsedSignature uses typing.get_type_hints which handles forward referencing but no TypeVar expansion. For instance,

from typing import TypeVar, Generic, get_type_hints

T = TypeVar("T")

class Foo(Generic[T]):
  def bar(self, data: T) -> T:
    pass 

genericAliasFoo = Foo[str]
print(get_type_hints(
  genericAliasFoo.bar, 
  globalns={"T": str}, 
  localns=None)) # gives {'data': ~T, 'return': ~T} 

This makes it difficult to write type generic handlers as there is no way of expanding TypeVar. Both pydantic and mypy use some forms of TypeVar expansion for the same scenario. A naive implementation can be as simple as this:

class ParsedSignature:
  @classmethod
  def from_fn(cls, fn: AnyCallable, signature_namespace: dict[str, Any]) -> Self:
      signature = Signature.from_callable(fn)
      fn_type_hints = get_fn_type_hints(fn, namespace=signature_namespace)

      # Expand type var
      for param, value in fn_type_hints.items():
        if value.__name__ in signature_namespace and isinstance(value, TypeVar):
          fn_type_hints[param] = signature_namespace[value]
      return cls.from_signature(signature, fn_type_hints)

Basic Example

As an example, to build a generic controller, this is what works currently (from #1311 and #2162)

class GenericController(Controller, Generic[T]):
    model_type: type[T]

    def __class_getitem__(cls, model_type: type[T]) -> type:
        return type(
            f"Controller[{model_type.__name__}]", (cls,), {"model_type": model_type}
        )

    def __init__(self, owner: Router):
        super().__init__(owner)
        self.signature_namespace[T.__name__] = self.model_type


class BaseController(GenericController[T]):
    @post()
    async def create(self, data: T.__name__) -> T.__name__:
        return data

Note how satanic the post handler looks. This works because T.__name__ is resolved using ForwardRef in get_type_hints. Under the new proposal, it is possible to do this instead:

class BaseController(GenericController[T]):
    @post()
    async def create(self, data: T) -> T:
        return data

Drawbacks and Impact

No response

Unresolved questions

No response


Note

While we are open for sponsoring on GitHub Sponsors and
OpenCollective, we also utilize Polar.sh to engage in pledge-based sponsorship.

Check out all issues funded or available for funding on our Polar.sh dashboard

  • If you would like to see an issue prioritized, make a pledge towards it!
  • We receive the pledge once the issue is completed & verified
  • This, along with engagement in the community, helps us know which features are a priority to our users.
Fund with Polar
@harryle95 harryle95 added the Enhancement This is a new feature or request label Mar 15, 2024
@peterschutt
Copy link
Contributor

We have

def get_type_hints_with_generics_resolved(
annotation: Any,
globalns: dict[str, Any] | None = None,
localns: dict[str, Any] | None = None,
include_extras: bool = False,
type_hints: dict[str, Any] | None = None,
) -> dict[str, Any]:
"""Get the type hints for the given object after resolving the generic types as much as possible.
Args:
annotation: A type annotation.
globalns: The global namespace.
localns: The local namespace.
include_extras: A flag indicating whether to include the ``Annotated[T, ...]`` or not.
type_hints: Already resolved type hints
"""
origin = get_origin(annotation)
if origin is None:
# Implies the generic types have not been specified in the annotation
if type_hints is None: # pragma: no cover
type_hints = get_type_hints(annotation, globalns=globalns, localns=localns, include_extras=include_extras)
typevar_map = {p: p for p in annotation.__parameters__}
else:
if type_hints is None: # pragma: no cover
type_hints = get_type_hints(origin, globalns=globalns, localns=localns, include_extras=include_extras)
# the __parameters__ is only available on the origin itself and not the annotation
typevar_map = dict(zip(origin.__parameters__, get_args(annotation)))
return {n: _substitute_typevars(type_, typevar_map) for n, type_ in type_hints.items()}

@harryle95
Copy link
Contributor Author

We have

def get_type_hints_with_generics_resolved(
annotation: Any,
globalns: dict[str, Any] | None = None,
localns: dict[str, Any] | None = None,
include_extras: bool = False,
type_hints: dict[str, Any] | None = None,
) -> dict[str, Any]:
"""Get the type hints for the given object after resolving the generic types as much as possible.
Args:
annotation: A type annotation.
globalns: The global namespace.
localns: The local namespace.
include_extras: A flag indicating whether to include the ``Annotated[T, ...]`` or not.
type_hints: Already resolved type hints
"""
origin = get_origin(annotation)
if origin is None:
# Implies the generic types have not been specified in the annotation
if type_hints is None: # pragma: no cover
type_hints = get_type_hints(annotation, globalns=globalns, localns=localns, include_extras=include_extras)
typevar_map = {p: p for p in annotation.__parameters__}
else:
if type_hints is None: # pragma: no cover
type_hints = get_type_hints(origin, globalns=globalns, localns=localns, include_extras=include_extras)
# the __parameters__ is only available on the origin itself and not the annotation
typevar_map = dict(zip(origin.__parameters__, get_args(annotation)))
return {n: _substitute_typevars(type_, typevar_map) for n, type_ in type_hints.items()}

I see, but fn_type_hints no longer contains the expanded type information after line 214. In the previous example, fn_type_hint = {'data': ~T, 'return': ~T}

fn_type_hints = get_fn_type_hints(fn, namespace=signature_namespace)

@peterschutt
Copy link
Contributor

Not sure what you mean. I'm just pointing out that we have a utility that does type get_type_hints() with type var expansion.

@harryle95
Copy link
Contributor Author

Sorry, I wasn't clear. Correct me if I am wrong, but I don't think that utility method gets called in the ParsedSignature.from_fn call stack.

@peterschutt
Copy link
Contributor

Obviously it was me who was unclear. I wasn't saying it did - just pointing out we have the utility you mentioned which maybe we can use. LMK how it goes.

@harryle95
Copy link
Contributor Author

Ah thank you. Should I make a PR after adding it in? Sorry I am quite new to contributing to open source project.

@peterschutt
Copy link
Contributor

You are more than welcome to make a PR, cheers.

To make it easier to review I'd first suggest creating a minimal example (I'm talking about something we can copy and paste and run without any extra work) that demonstrates the issue against main. Then we have some context when reviewing the PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement This is a new feature or request
Projects
None yet
2 participants