Skip to content

Comments

[ty] print function names for bare Signatures opportunistically#21438

Closed
Gankra wants to merge 5 commits intomainfrom
gankra/signature-print
Closed

[ty] print function names for bare Signatures opportunistically#21438
Gankra wants to merge 5 commits intomainfrom
gankra/signature-print

Conversation

@Gankra
Copy link
Contributor

@Gankra Gankra commented Nov 13, 2025

This is a followup to #21417 that improves some places where we pretty-display types (hence being restricted to multiline printing). Even in cases where we can't get the name it behooves us to add a def _ to fix python syntax highlighting in e.g. hovers.

@Gankra Gankra added the server Related to the LSP server label Nov 13, 2025
@Gankra Gankra added the ty Multi-file analysis & type inference label Nov 13, 2025
@astral-sh-bot
Copy link

astral-sh-bot bot commented Nov 13, 2025

Diagnostic diff on typing conformance tests

No changes detected when running ty on typing conformance tests ✅

@astral-sh-bot
Copy link

astral-sh-bot bot commented Nov 13, 2025

mypy_primer results

No ecosystem changes detected ✅

No memory usage changes detected ✅

@Gankra
Copy link
Contributor Author

Gankra commented Nov 13, 2025

With the two PRs combined, stdlib calls become readable on hover, if a bit innaccurate about methods:

Screenshot 2025-11-13 at 4 35 24 PM

@codspeed-hq
Copy link

codspeed-hq bot commented Nov 13, 2025

CodSpeed Performance Report

Merging #21438 will improve performances by 5.08%

Comparing gankra/signature-print (cfd7591) with gankra/overloads (dcc451d)

Summary

⚡ 2 improvements
✅ 20 untouched
⏩ 30 skipped1

Benchmarks breakdown

Mode Benchmark BASE HEAD Change
Simulation ty_micro[complex_constrained_attributes_1] 70.1 ms 67 ms +4.59%
Simulation ty_micro[complex_constrained_attributes_2] 70.3 ms 66.9 ms +5.08%

Footnotes

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

@carljm
Copy link
Contributor

carljm commented Nov 14, 2025

Signatures don't necessarily come from def statements (or functions), so I feel like it's kind of misleading to prepend def _ arbitrarily to signature rendering. I understand that this gives us syntax highlighting for free :/ Probably worth it, but unfortunate.

{
writer.write_str(&name)?;
} else {
writer.write_str("_")?;
Copy link
Member

@MichaReiser MichaReiser Nov 16, 2025

Choose a reason for hiding this comment

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

What's an example where we don't have a name? Is it for Callable types?

Pylance uses a different rendering for callable types:

Screenshot 2025-11-16 at 09 08 40

Which I think I prefer, as it emphasizes the fact that one is a callable rather than a specific function instance

Copy link
Member

Choose a reason for hiding this comment

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

Yes, it's for Callable and lambda expressions and we do use the same display approach.

@dhruvmanila
Copy link
Member

I'll put this in draft for now as this might not be necessary, refer to my comment here: #21417 (comment).

@dhruvmanila dhruvmanila marked this pull request as draft November 17, 2025 08:33
Base automatically changed from gankra/overloads to main November 20, 2025 17:45
@Gankra
Copy link
Contributor Author

Gankra commented Nov 21, 2025

Yeah I'm gonna send this to hell for now

@Gankra Gankra closed this Nov 21, 2025
Gankra added a commit that referenced this pull request Dec 17, 2025
This is the return of #21438 because we never found anything better and
I think it would be good to have this for the beta.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

server Related to the LSP server ty Multi-file analysis & type inference

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants