-
Notifications
You must be signed in to change notification settings - Fork 1.6k
[ty] Synthetic function-like callables #18242
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
Conversation
|
0016661 to
111b0a9
Compare
111b0a9 to
5ca8ace
Compare
hmm, it seems incorrect that we're emitting this diagnostic. It definitely exists at runtime: >>> def f(): ...
...
>>> f.__code__
<code object f at 0x105d60b90, file "<python-input-0>", line 1>
>>> class Foo:
... def f(): ...
...
>>> Foo().f.__code__
<code object f at 0x105d60c60, file "<python-input-2>", line 2>and it's annotated as existing on ruff/crates/ty_vendored/vendor/typeshed/stdlib/types.pyi Lines 72 to 76 in cb9e669
I suppose we need to fallback to |
|
Are |
|
I guess I'm not totally sold that these should be modeled as a special case of |
|
I think it would be important to see some new subtyping and assignability tests (and the code to make them pass, which I also don't think I'm seeing here?), which should verify several things:
|
| /// We use `CallableType` to represent function-like objects, like the synthesized methods | ||
| /// of dataclasses or NamedTuples. These callables act like real functions when accessed | ||
| /// as attributes on instances, i.e. they bind `self`. |
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 think we could be a bit more precise in our description here, i.e. a callable type with is_function_like: True is inhabited by callable objects with the given signature that also have a __get__ method which acts like FunctionType.__get__ (returns a bound method).
Yes, thanks! At runtime, it also exists for generated methods on dataclasses. This should be fixed now. |
Yes, thanks. Added the test, but haven't fixed it yet.
I originally planned it this way, but somehow accidentally did the "reverse" today and special-cased |
So this is interesting. I added the tests, and they.. just pass. It works not because we're looking at the type DunderInitType = TypeOf[C.__init__] # (self: C, x: int) -> None
type EquivalentCallableType = Callable[[C, int], None] # (C, int, /) -> NoneThis is probably not a great way to ensure proper subtyping/assignability properties, so I can look into incorporating |
I think the implementation shouldn't be too difficult? Just a check in Writing a failing test may be more of the challenge, but I think if you also define |
it being "easier" doesn't necessarily reassure me ;) that might just mean that we've accidentally omitted some branches in some places that we would otherwise be forced to add if it was its own variant 😆 |
|
If we do have this as a form of Callable type, it might mean that we could be "smart" about On the other hand, it's weird to interpret the same annotation differently depending on the RHS. So maybe we shouldn't do that. |
bb339b4 to
3e5f17d
Compare
3e5f17d to
eaab5d1
Compare
|
Opening this up for review again. It passes all tests and resolves some ecosystem false positives. I am happy to spend (significantly?) more time on this, trying to rewrite it in terms of a customized |
carljm
left a comment
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 think it's fine to go with this for now.
But I think we should use function-like callables for lambdas too, since they're also instances of |
* main: [ty] Support ephemeral uv virtual environments (#18335) Add a `ViolationMetadata::rule` method (#18234) Return `DiagnosticGuard` from `Checker::report_diagnostic` (#18232) [flake8_use_pathlib]: Replace os.symlink with Path.symlink_to (PTH211) (#18337) [ty] Support cancellation and retry in the server (#18273) [ty] Synthetic function-like callables (#18242) [ty] Support publishing diagnostics in the server (#18309) Add Autofix for ISC003 (#18256) [`pyupgrade`]: new rule UP050 (`useless-class-metaclass-type`) (#18334) [pycodestyle] Make `E712` suggestion not assume a context (#18328)
* main: (246 commits) [ty] Simplify signature types, use them in `CallableType` (astral-sh#18344) [ty] Support ephemeral uv virtual environments (astral-sh#18335) Add a `ViolationMetadata::rule` method (astral-sh#18234) Return `DiagnosticGuard` from `Checker::report_diagnostic` (astral-sh#18232) [flake8_use_pathlib]: Replace os.symlink with Path.symlink_to (PTH211) (astral-sh#18337) [ty] Support cancellation and retry in the server (astral-sh#18273) [ty] Synthetic function-like callables (astral-sh#18242) [ty] Support publishing diagnostics in the server (astral-sh#18309) Add Autofix for ISC003 (astral-sh#18256) [`pyupgrade`]: new rule UP050 (`useless-class-metaclass-type`) (astral-sh#18334) [pycodestyle] Make `E712` suggestion not assume a context (astral-sh#18328) put similar dunder-call tests next to each other (astral-sh#18343) [ty] Derive `PartialOrd, Ord` for `KnownInstanceType` (astral-sh#18340) [ty] Simplify `Type::try_bool()` (astral-sh#18342) [ty] Simplify `Type::normalized` slightly (astral-sh#18339) [ty] Move arviz off the list of selected primer projects (astral-sh#18336) [ty] Add --config-file CLI arg (astral-sh#18083) [ty] Tell the user why we inferred a certain Python version when reporting version-specific syntax errors (astral-sh#18295) [ty] Implement implicit inheritance from `Generic[]` for PEP-695 generic classes (astral-sh#18283) [ty] Add hint if async context manager is used in non-async with statement (astral-sh#18299) ...
Summary
We create
Callabletypes for synthesized functions like the__init__method of a dataclass. These generated functions are real functions though, with descriptor-like behavior. That is, they can bindselfwhen accessed on an instance. This was modeled incorrectly so far.Test Plan
Updated tests