Skip to content

Conversation

@Glyphack
Copy link
Contributor

@Glyphack Glyphack commented Jun 4, 2025

Summary

Part of astral-sh/ty#159

Add support for adding a synthetic typing.Self type for self arguments in methods.
typing.Self is assigned as the type if there are no annotations.
This PR only adds the functionality when the symbol lookup is happening.

#18007 is handling the case when a call is happening.

Surfaced issues

After running tests and mypy primer some tests started failing because self was not Any anymore:

MDtests

Too many cycle iterations

class ChunkedBytesIO:
    def __init__(self: Self, contents: list[bytes]) -> None:
        self.contents = contents
        self.pos = (0, 0)

    def read(self: Self):
        chunk, cursor = self.pos
        self.pos = (chunk + 1, cursor)

Test Plan

  • Updated md tests.
  • Added new TODOs because some tests started failing once the self was not unknown anymore.

@ntBre ntBre added the ty Multi-file analysis & type inference label Jun 4, 2025
@Glyphack Glyphack force-pushed the typing-self-function-scope branch 2 times, most recently from 5b6005e to 23d7a60 Compare June 5, 2025 21:01
@dcreager
Copy link
Member

dcreager commented Jun 6, 2025

Hi @Glyphack, saw that you opened this up! Let me know if you want to find some time to chat through the options.

Edit: Tomorrow, that is, it's the end of the day for me 😄

@Glyphack Glyphack force-pushed the typing-self-function-scope branch 2 times, most recently from 3d924df to c910aa5 Compare June 11, 2025 21:09
@codspeed-hq
Copy link

codspeed-hq bot commented Jun 11, 2025

CodSpeed Performance Report

Merging #18473 will degrade performances by 16.08%

Comparing Glyphack:typing-self-function-scope (a4d6a2a) with main (058fc37)1

Summary

❌ 6 regressions
✅ 42 untouched
⏩ 4 skipped2

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Benchmarks breakdown

Mode Benchmark BASE HEAD Change
Instrumentation anyio 819.6 ms 915.2 ms -10.44%
WallTime large[sympy] 38.4 s 41.9 s -8.26%
WallTime small[altair] 2.3 s 2.4 s -4.26%
WallTime small[freqtrade] 4.2 s 4.8 s -12.64%
WallTime small[pydantic] 2.4 s 2.5 s -7.4%
WallTime small[tanjun] 1.6 s 1.9 s -16.08%

Footnotes

  1. No successful run was found on david/type-of-self-in-methods-integration (ec9faa3) during the generation of this report, so main (058fc37) was used instead as the comparison base. There might be some changes unrelated to this pull request in this report.

  2. 4 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@github-actions

This comment was marked as resolved.

@Glyphack Glyphack marked this pull request as ready for review June 11, 2025 22:02
@MichaReiser MichaReiser changed the title Infer type of self as typing.Self in method body [ty] Infer type of self as typing.Self in method body Jun 12, 2025
@Glyphack Glyphack requested a review from MichaReiser as a code owner June 14, 2025 13:02
@Glyphack Glyphack force-pushed the typing-self-function-scope branch from e0cdb98 to 536b734 Compare June 14, 2025 13:19
@Glyphack Glyphack marked this pull request as draft June 17, 2025 07:02
@Glyphack Glyphack force-pushed the typing-self-function-scope branch 2 times, most recently from 384d96c to 1a31240 Compare June 21, 2025 22:12
@Glyphack

This comment was marked as resolved.

sharkdp added a commit that referenced this pull request Sep 29, 2025
Part of astral-sh/ty#159

This PR only adjusts the signature of a method so if it has a `self`
argument then that argument will have type of `Typing.Self` even if it's
not specified. If user provides an explicit annotation then Ty will not
override that annotation.

- astral-sh/ty#1131
- astral-sh/ty#1157
- astral-sh/ty#1156
- astral-sh/ty#1173
- #20328
- astral-sh/ty#1163
- astral-sh/ty#1196

Added mdtests.
Also some tests need #18473 to
work completely. So I added a todo for those new cases that I added.

---------

Co-authored-by: David Peter <mail@david-peter.de>
sharkdp added a commit that referenced this pull request Sep 29, 2025
Part of astral-sh/ty#159

This PR only adjusts the signature of a method so if it has a `self`
argument then that argument will have type of `Typing.Self` even if it's
not specified. If user provides an explicit annotation then Ty will not
override that annotation.

- astral-sh/ty#1131
- astral-sh/ty#1157
- astral-sh/ty#1156
- astral-sh/ty#1173
- #20328
- astral-sh/ty#1163
- astral-sh/ty#1196

Added mdtests.
Also some tests need #18473 to
work completely. So I added a todo for those new cases that I added.

---------

Co-authored-by: David Peter <mail@david-peter.de>
sharkdp added a commit that referenced this pull request Sep 29, 2025
Part of astral-sh/ty#159

This PR only adjusts the signature of a method so if it has a `self`
argument then that argument will have type of `Typing.Self` even if it's
not specified. If user provides an explicit annotation then Ty will not
override that annotation.

- astral-sh/ty#1131
- astral-sh/ty#1157
- astral-sh/ty#1156
- astral-sh/ty#1173
- #20328
- astral-sh/ty#1163
- astral-sh/ty#1196

Added mdtests.
Also some tests need #18473 to
work completely. So I added a todo for those new cases that I added.

---------

Co-authored-by: David Peter <mail@david-peter.de>
@MichaReiser
Copy link
Member

MichaReiser commented Oct 11, 2025

I "forked" this PR in #20812 and updated the salsa version. There are 4 new projects that now fail with too many iteration errors https://micha-typing-self-function-s.ecosystem-663.pages.dev/timing. I only checked setuptools, and the error looks legit. The type becomes massive! I think this should now be unblocked (once we figure out how to fix the non-convergent cases).

The perf impact is still pretty "bad" but I think that's expected given that we now check much more code https://codspeed.io/astral-sh/ruff/branches/micha%2Ftyping-self-function-scope

@sharkdp sharkdp changed the base branch from main to david/type-of-self-in-methods-integration October 16, 2025 14:30
@sharkdp
Copy link
Contributor

sharkdp commented Oct 16, 2025

I will squash-merge this into #20922 for the same reasons that applied for the previous type-of-self PR. I'd like to clean up the looong history a bit (I will transfer comments that are still relevant), and I'd like to iterate faster on the ecosystem results, which are more convenient to use when it's not a contributor PR. @Glyphack thank you!

@sharkdp sharkdp marked this pull request as ready for review October 16, 2025 14:38
@sharkdp sharkdp merged commit c6cfaf7 into astral-sh:david/type-of-self-in-methods-integration Oct 16, 2025
37 of 40 checks passed
sharkdp added a commit that referenced this pull request Oct 22, 2025
Part of astral-sh/ty#159

Add support for adding a synthetic `typing.Self` type for `self`
arguments in methods.
`typing.Self` is assigned as the type if there are no annotations.

- Updated md tests.

---------

Co-authored-by: David Peter <mail@david-peter.de>
@Glyphack Glyphack deleted the typing-self-function-scope branch October 22, 2025 15:12
sharkdp added a commit that referenced this pull request Oct 23, 2025
Part of astral-sh/ty#159

Add support for adding a synthetic `typing.Self` type for `self`
arguments in methods.
`typing.Self` is assigned as the type if there are no annotations.

- Updated md tests.

---------

Co-authored-by: David Peter <mail@david-peter.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ecosystem-analyzer ty Multi-file analysis & type inference

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants