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
Copy link

codspeed-hq bot commented Jun 11, 2025

CodSpeed Performance Report

Merging #18473 will degrade performances by 11.77%

Comparing Glyphack:typing-self-function-scope (13457d2) with main (23ebfe7)

Summary

❌ 1 regression
✅ 42 untouched
⏩ 9 skipped1

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

Benchmarks breakdown

Mode Benchmark BASE HEAD Change
Instrumentation anyio 873.9 ms 990.5 ms -11.77%

Footnotes

  1. 9 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.

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 23, 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 <[email protected]>
@MichaReiser
Copy link
Member

I ran the walltime benchamarks locally after pulling in #20515

This PR clearly regresses for all projects but they complete in a reasonable time:

# main
Main

Timer precision: 41 ns
ty_walltime           fastest       │ slowest       │ median        │ mean          │ samples │ iters
├─ large                            │               │               │               │         │
│  ╰─ sympy           5.032 s       │ 5.34 s        │ 5.186 s       │ 5.186 s       │ 2       │ 2
├─ medium                           │               │               │               │         │
│  ├─ colour-science  1.18 s        │ 1.235 s       │ 1.192 s       │ 1.202 s       │ 3       │ 3
│  ├─ pandas          3.765 s       │ 3.893 s       │ 3.812 s       │ 3.823 s       │ 3       │ 3
│  ╰─ static-frame    1.073 s       │ 1.182 s       │ 1.099 s       │ 1.118 s       │ 3       │ 3
├─ multithreaded                    │               │               │               │         │
│  ╰─ pydantic        58.27 ms      │ 60.06 ms      │ 59.26 ms      │ 59.17 ms      │ 8       │ 24
╰─ small                            │               │               │               │         │
   ├─ altair          335.2 ms      │ 342.4 ms      │ 340.3 ms      │ 339.3 ms      │ 3       │ 6
   ├─ freqtrade       580.5 ms      │ 601.1 ms      │ 587.4 ms      │ 589.7 ms      │ 3       │ 6
   ├─ pydantic        301.2 ms      │ 309.1 ms      │ 305.1 ms      │ 305.1 ms      │ 3       │ 6
   ╰─ tanjun          215.6 ms      │ 229.3 ms      │ 218.7 ms      │ 221.2 ms      │ 3       │ 6

# self type
ty_walltime           fastest       │ slowest       │ median        │ mean          │ samples │ iters
├─ large                            │               │               │               │         │
│  ╰─ sympy           6.35 s        │ 6.801 s       │ 6.575 s       │ 6.575 s       │ 2       │ 2
├─ medium                           │               │               │               │         │
│  ├─ colour-science  1.204 s       │ 1.218 s       │ 1.205 s       │ 1.209 s       │ 3       │ 3
│  ├─ pandas          5.059 s       │ 5.081 s       │ 5.068 s       │ 5.069 s       │ 3       │ 3
│  ╰─ static-frame    1.133 s       │ 1.137 s       │ 1.136 s       │ 1.135 s       │ 3       │ 3
├─ multithreaded                    │               │               │               │         │
│  ╰─ pydantic        60.82 ms      │ 62.54 ms      │ 62.03 ms      │ 61.89 ms      │ 8       │ 24
╰─ small                            │               │               │               │         │
   ├─ altair          329 ms        │ 333.1 ms      │ 331.1 ms      │ 331 ms        │ 3       │ 6
   ├─ freqtrade       626.1 ms      │ 645.8 ms      │ 633.3 ms      │ 635.1 ms      │ 3       │ 6
   ├─ pydantic        318.1 ms      │ 325 ms        │ 320.5 ms      │ 321.2 ms      │ 3       │ 6
   ╰─ tanjun          247.3 ms      │ 249.3 ms      │ 248.3 ms      │ 248.3 ms      │ 3       │ 6

@MichaReiser
Copy link
Member

MichaReiser commented Sep 23, 2025

The mypy primer runs still take significantly longer:

/tmp/mypy_primer/ty_old/target/debug/ty on prefect took 4.94s
/tmp/mypy_primer/ty_new/target/debug/ty on prefect took 26.98s

/tmp/mypy_primer/ty_old/target/debug/ty on apprise took 1.97s
/tmp/mypy_primer/ty_new/target/debug/ty on apprise took 34.79s

and there are projects that still don't complete, e.g. parso

But it's certainly much better than before where prefect never completed too

One of the files that's much slower on this branch than before is src/prefect/_vendor/croniter/croniter.py

sharkdp added a commit that referenced this pull request Sep 24, 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 <[email protected]>
sharkdp added a commit that referenced this pull request Sep 25, 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 <[email protected]>
sharkdp added a commit that referenced this pull request Sep 25, 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 <[email protected]>
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 <[email protected]>
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 <[email protected]>
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 <[email protected]>
@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

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