Skip to content

Ensure that GenericCallable is generated if typevars are present.#91

Closed
dnwpark wants to merge 7 commits intomainfrom
external-typevar
Closed

Ensure that GenericCallable is generated if typevars are present.#91
dnwpark wants to merge 7 commits intomainfrom
external-typevar

Conversation

@dnwpark
Copy link
Contributor

@dnwpark dnwpark commented Feb 10, 2026

Currently, we don't properly generate GenericCallable if a typevar in the signature is from outside a class. For example:

T = TypeVar('T')
class C:
    @staticmethod
    def f(x: T) -> T: ...

Previously, C.f would be typed as Callable[Param[Literal["x"], T], T] while it should correctly be typed something like GenericCallable[tuple[T], lambda T: Callable[Param[Literal["x"], T], T].

@dnwpark dnwpark requested a review from msullivan February 10, 2026 01:50
@vercel
Copy link

vercel bot commented Feb 10, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
python-typemap Ready Ready Preview, Comment Feb 12, 2026 5:30pm

@dnwpark dnwpark changed the base branch from main to generic-callable February 10, 2026 01:50
@msullivan msullivan force-pushed the generic-callable branch 2 times, most recently from f6745ce to a3cb55c Compare February 10, 2026 02:50
Base automatically changed from generic-callable to main February 10, 2026 21:47
ordinary: str
def foo(self: Self, a: int | None, *, b: int = ...) -> dict[str, int]: ...
def base[Z](self: Self, a: int | Z | None, b: ~K) -> dict[str, int | Z]: ...
def base[Z, K](self: Self, a: int | Z | None, b: ~K) -> dict[str, int | Z]: ...
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe we should take that K out of Final. It doesn't really make sense.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

other than the wacky formatting (K vs ~K), what would you expect this to look like?

Comment on lines +539 to +541
def member_method(
self, x: T
) -> set[T] if IsAssignable[T, int] else T: ...
Copy link
Collaborator

Choose a reason for hiding this comment

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

This case doesn't work in general, obviously, since one side could have a type var that the other doesn't.

We should at least document that somewhere.

I actually do have an insane plan that would allow solving this, involving repeatedly reexecuting the annotation with Bool et al rigged up to return different results, but Yury was horrified by the idea and we probably shouldn't do it.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Another wild approach would be to read the bytecode and look for all globals that might be read.

Comment on lines +295 to +296
# - pattern matched annotations: T | None, set[T], etc.
# - type vars in an expression: U if IsAssignable[T, int] else V
Copy link
Collaborator

Choose a reason for hiding this comment

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

The first one we definitely need to handle, if we are handling any cases at all.

The second one we can hopefully ignore? At least, the solutions are all crazy.

Comment on lines +389 to +392
for tv in _find_annotation_type_vars(raw_args, rr, globs):
if tv not in type_params:
type_params.append(tv)
rr = _eval_raw_annotations(raw_args, rr, globs)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think I understand the algorithm here. Is the split where we try _find_annotation_type_vars before _eval_raw_annotations being done just to support the case with an if or for?

If so I think maybe we should skip doing that and just try processing the result of _eval_raw_annotations, since we can't make for/if work without doing some horrible things in general...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The goal is to leave the bulk of the logic identical for get_annotations. At the same time, I break it into parts so that I am able to read the annotations that are used in intermediate computations. This is to find any type vars that were not declared by the function itself.

These new type vars are added to those declared by the function when generating the GenericCallable lambda.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This has nothing to do with for/if in particular.

Copy link
Collaborator

Choose a reason for hiding this comment

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

OK, I guess the question here is (and I thought I half understood the answer): why can't we just call get_annotations and look for all the type variables in the result?

Copy link
Contributor Author

@dnwpark dnwpark Feb 12, 2026

Choose a reason for hiding this comment

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

The short answer is that at this point in get_local_defns, we call get_annotations, which may return None if it gets stuck. In that situation, there is no result to look into.

It gets stuck because the externally defined type var doesn't get substituted anywhere and so results in the type var itself.

We want to detect all the type vars while also avoiding the stuck exception. Some ways I thought about doing this:

  • some sort of flag in the execution context that supresses stuck and then also store things in the context
  • a different type of execution context entirely
  • maybe looking at the annotate function globals

Copy link
Contributor Author

@dnwpark dnwpark Feb 12, 2026

Choose a reason for hiding this comment

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

Ok also, we may or may not need to cover external type vars that are in if/for, but I was more thinking that if any annotation has if/for, then this will occur, even if the external type var is not in some complex expression

eg.

T = typevar("T")
def f(self, x: T, y: int if IsAssignable[T,int] else str) -> T

return f


def _find_function_type_vars(ty, *, seen=None):
Copy link
Collaborator

Choose a reason for hiding this comment

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

This probably should recurse, right? Through all typealiases

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, but I wanted to keep this in sync with _eval_operators._collect_type_vars, which I didn't want to figure out yet.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants