Skip to content

Typing tweaks#547

Merged
alcarney merged 2 commits into
openlawlibrary:mainfrom
dimbleby:partial-typing-tweaks
Jun 6, 2025
Merged

Typing tweaks#547
alcarney merged 2 commits into
openlawlibrary:mainfrom
dimbleby:partial-typing-tweaks

Conversation

@dimbleby

@dimbleby dimbleby commented Jun 5, 2025

Copy link
Copy Markdown
Contributor

the typing enhancements around get_capability in 2.0.0a4 are I think not quite right. Here is their effect in jedi-language-server

jedi_language_server/server.py:99: error: Module "pygls.capabilities" does not explicitly export attribute "get_capability"  [attr-defined]
jedi_language_server/server.py:338: error: Argument "enable_snippets" to "lsp_completion_item" has incompatible type "Optional[bool]"; expected "bool"  [arg-type]
jedi_language_server/server.py:1127: error: Unsupported right operand type for in ("Optional[Sequence[MarkupKind]]")  [operator]
jedi_language_server/server.py:1128: error: Value of type "Optional[Sequence[MarkupKind]]" is not indexable  [index]

re-exporting get_capability is easy and is the first commit here.

However I think that the overloads are also not quite right:

  • I do not understand what the type variable T is for?
  • the types say that even though the caller has provided a default, None might be returned - which should not be the case

In the second commit I have updated only the two overloads that are relevant in jedi-language-server, but if you agree that this is the correct direction then hopefully it is clear what the pattern is.

@dimbleby

dimbleby commented Jun 5, 2025

Copy link
Copy Markdown
Contributor Author

oh I see, this code is generated. pull request updated accordingly

@dimbleby dimbleby changed the title Partial typing tweaks Typing tweaks Jun 5, 2025
@alcarney

alcarney commented Jun 6, 2025

Copy link
Copy Markdown
Collaborator

This makes a lot more sense - thank you!

I do not understand what the type variable T is for?

I think I was trying to express that you could, in theory at least, pass any value to the default parameter. However, when you think about it you're only ever going to want to pass a value that's compatible with the capability you are checking for, so simplifying it makes a lot of sense.

Do you need this released in a 2.0a5 release now? Or is it ok to wait a little while?
Aside from potential bug fixes I'm not planning on making any major changes at this point, but I was tentatively thinking of formally dropping Python 3.9 in 2.0a5 ready for October.

@alcarney alcarney merged commit be50d2c into openlawlibrary:main Jun 6, 2025
18 checks passed
@dimbleby

dimbleby commented Jun 6, 2025

Copy link
Copy Markdown
Contributor Author

no need for another release so far as I am concerned, thanks.

No objection from me to dropping python 3.9 once it is eol.

(Though, unless there is something about supporting 3.9 that is holding you back in some way - and I would accept that a very small reason was a good enough reason! - well I wonder whether sometimes projects are a bit fast to drop support that they didn't actually need to drop. Whatever.)

The one thing I would like to see before a non-pre-release release is that dependencies - in particular lsprotocol - should also be at non-pre-release versions. Per #542, I expect that should happen.

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