Skip to content
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

Improve method annotations #259

Closed
wants to merge 1 commit into from

Conversation

layday
Copy link
Contributor

@layday layday commented Apr 26, 2022

When registering an unstructure hook the type checker is not able to
infer the type of T; Pyright considers its type to be unknown
(eq. to object). When attempting to call a method on T a type error
is produced.

When registering an unstructure hook the type checker is not able to
infer the type of `T`; Pyright considers its type to be unknown
(eq. to object).  When attempting to call a method on `T` a type error
is produced.
@Tinche
Copy link
Member

Tinche commented Apr 26, 2022

Some special types like unions and optionals aren't covered by type[T], so these cannot be annotated like this. They used to be, a few versions back, but I had to revert it. We'd need something like python/mypy#9773 to get accepted and merged first.

@Tinche Tinche closed this Apr 26, 2022
@Tinche
Copy link
Member

Tinche commented Apr 26, 2022

See also #111

@layday
Copy link
Contributor Author

layday commented Apr 26, 2022 via email

@Tinche
Copy link
Member

Tinche commented Apr 27, 2022

Sure, where would you like the Any?

@layday
Copy link
Contributor Author

layday commented Apr 30, 2022

In register_unstructure_hook, replacing the stand-alone type variable. It would also be helpful to annotate the unstructure_as parameter of unstructure with Optional[Any].

@Tinche
Copy link
Member

Tinche commented Apr 30, 2022

I tweaked the annotations. I don't think Optional[Any] makes sense though (Any already contains None), so I just annotated it as Any.

@layday
Copy link
Contributor Author

layday commented May 1, 2022

Great, thanks!

I don't think Optional[Any] makes sense though (Any already contains None)

Well, it conveys intent, even if Optional[Any] < Any.

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