-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
✨ Add autocomplete support for VS Code, via dataclass_transform
#2721
Conversation
Oh! I saw the proposal but didn't see it was already supported! That's great!! |
Probably worth noting that this imposes some restrictions on the constructor as it is currently implemented in Pyright, requiring exact types to be passed and forbidding extra values. It also doesn't have knowledge of some Pydantic conventions, e.g. |
Yeah, this actually doesn't impose any restriction on Pydantic itself, it's not really used by pydantic internally, it only tells Pyright that it can provide completion for pydantic models as if they were dataclasses. But any extra data passed to the constructor would still be received by pydantic, and, e.g. dicts for pydantic submodels would still be converted. You can try it right away, open VS Code and use it there, it's a small modification to the pydantic source code and then you can import from that and see how it works locally. |
Ah, get it, that's right. I normally have Pyright type checks disabled and only use mypy for the type checks, so I didn't see that one. But yeah, that would make sense. Still, I think this is an improvement over the default editor support of just having Maybe then another extension could be added on top with the specific details for Pydantic, but until then (and I imagine a lot of time could pass until then), having this default "like-dataclasses" support would help a lot and would work well for a large number of use cases. |
Yeah, I agree, it's a net plus, and you can bypass the strict type check by unpacking a dict or via |
This seems great to me, it reminds me I need to review the proposal and also reply to the python-dev thread in support of it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
other all I think this looks good, but would be great to get @erictraut's feedback.
@@ -223,6 +237,7 @@ def hash_function(self_: Any) -> int: | |||
_is_base_model_class_defined = False | |||
|
|||
|
|||
@__dataclass_transform__(kw_only_default=True, field_descriptors=(Field, FieldInfo)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there any possibility to add the frozen
argument which should equal config.frozen or not config.allow_mutation
?
I can't see how this is possible but @erictraut suggested it should be.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can pass frozen=True when constructing a class that derives from ModelBase
, like this:
class CustomerModel(ModelBase, frozen=True):
That was based on a suggestion you made in our email discussion — or at least how I interpreted your suggestion. Did I understand you correctly?
All classes are assumed to be non-frozen unless otherwise specified. There is a request from the attrs
maintainers to provide a way to specify a frozen_default
option so they can default to frozen=True
if it is not specified. If that would also be useful for pydantic, it would bolster the case for adding it to the spec.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, that's awesome. I've just tried it and it's already working!
This looks great then, the only problem I can see is the type strictness/laxness which is being discussed on microsoft/pyright#1782
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
EDIT: Dang it, I didn't see all your previous conversation above this message, I guess I had a stale window. Sorry.
EDIT 2: discard this message pretty much entirely, already covered above and below. 🤦 😂
Old message below:
I didn't see a way to do it from what I read in the spec.
If I understand correctly, it would be possible for Pyright (and the spec) to understand if the model was created like:
from pydantic import BaseModel
class Fish(BaseModel, frozen=True):
title: str
...which is currently not supported by pydantic (although I think sounds interesting).
What is currently supported by pydantic (but I understand not by Pyright nor the spec) is:
from pydantic import BaseModel
class Fish(BaseModel):
title: str
class Config:
frozen = True
I'll wait for @erictraut's input in case it's possible to support pydantic's internal class Config
in a way I didn't realize.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
class Fish(BaseModel, frozen=True):
title: str
is actually supported in 1.8 @tiangolo 😉 Config arguments can now be passed as class kwargs thanks to @MrMrRobat's amazing work in #2356
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dang it again! 🤦 😂 pydantic has evolved so much and I haven't noticed everything! 😅
Thanks for the clarification @PrettyWood, that's awesome! 🎉
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried this with v1.9 and Pyright does not seem to recognise frozen
:
# pyright: strict
from typing import Hashable
from pydantic import BaseModel
class Foo(BaseModel, frozen=True):
pass
foo: Hashable = Foo() # Error: Expression of type "Foo" cannot be assigned to declared type "Hashable"
reveal_type(Foo.__hash__) # Type of "Foo.__hash__" is "None"
from dataclasses import dataclass
@dataclass(frozen=True)
class Bar:
pass
bar: Hashable = Bar() # No error
reveal_type(Bar.__hash__) # Type of "Bar.__hash__" is "(self: Bar) -> int"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting, this probably needs a new issue.
I've no idea if this is something pydantic can help with, or an issue with pyright. @erictraut any idea?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@layday you point about types is valid, there's an issue about this for the pycharm plugin: koxudaxi/pydantic-pycharm-plugin#36. I think the first step is this. That might help the pycharm plugin, but I don't see how it will ever be supported by the dataclass equivalence route. |
@tiangolo I think The style may not beautiful 🤔 But, the change doesn't affect runtime. |
Yes, I should resolve the problem. But, I have not touched on the problem yet because I'm I can change the implementation of the pydantic-pycharm-plugin easily. |
Thanks for the review @samuelcolvin ! And thanks for the help and clarifications @layday ! 🤓
Good point @koxudaxi ! 🤓 That means a bit of extra code and duplication that would save an extra function decorator with a lambda. I suspect the performance difference would not be much but would be something. 🤷 As the extra code and duplication would add some maintenance burden and the performance effect might not be huge, I'll wait for @samuelcolvin's opinion on that. Let me know if you want me to put that in a
We are all trying to do our best, and I think we are all just (and only) grateful for all your work on that. It has benefited us all, including a LOT of developers. 🙇 |
I agree it'll only be a small difference, I think here it's probably best to leave it in the simplest form possible. |
If I understand correctly, this is ready for review/merge. Otherwise, let me know if there's anything missing, or that you would want changed, etc. Also, I didn't add docs for this here. Should I? If there were docs for this, I imagine they would go right below the section/page for the PyCharm plugin, and they could have some or all of:
It could probably include some screenshots, although maybe you would prefer to do them yourself to keep consistency and screen proportions (I do that with FastAPI's docs). I can do that, or any subset of that if you would like. |
yes, I think some docs along the line of what you've suggested would be great. You're the king of docs @tiangolo, so I'm sure they will be great. 🙇 |
Thanks! 🤓 I added some docs now: https://smokeshow.helpmanual.io/2s422a4n6x4w0p451j5x/visual_studio_code/ I included several screenshots as some of the concepts are harder to describe with words but are quite intuitive to understand with images. |
docs/visual_studio_code.md
Outdated
|
||
that way Pylance/Pyright and mypy will interpret the variable `age_str` as if they didn't know its type, instead of knowing it has a type of `str` when an `int` was expected (and then showing the corresponding error). | ||
|
||
The advantage of this technique is that you will still see any additional errors for the other arguments. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, the advantage is that it will not disable all type checking for that line, e.g. it will alert you if you misspell age
or the name of the class or if you make a syntactical error. # type: ignore
disables all of these checks. A third option would also be to use cast
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point. And great idea! cast
could work quite well. 🚀
I'll add the cast
option and update the docs for that to explain the disadvantages of # type: ignore
as well. And show the 3 alternatives in order of increasing steps/complexity.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cast
should work nicely but as I mentioned in the discussion, the problem is that is technically wrong.
cast()
says "this think is actually an int
, promise. If I'm wrong, it's my problem" here that's not the case, the thing really is a string (or whatever) it's just that that type is valid.
I'm afraid sadly there's no non-hack here, but I guess cast()
could be useful.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The non-hack is to be explicit in type conversions. If pydantic contains logic that performs type conversions implicitly, does it expose those type conversion routines so they can be invoked explicitly for users who are interested in static type safety? The use of Any
or cast
or # type: ignore
are all poor workarounds if you care about static type checking.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cast
should work nicely but as I mentioned in the discussion, the problem is that is technically wrong. [...]
Yep, agreed. I updated the docs including another example using cast(Any, '23')
instead of cast(int, '23')
, I think that's an acceptable balance, telling the editor "don't check this", which would also work for any other type, without having to do something that is technically wrong like cast(int, '23')
.
The non-hack is to be explicit in type conversions. [...]
Agreed, and for cases like the example with a '23'
it's quite obvious and it would be a lot better to do the conversion manually.
But for things like datetime
s that accept multiple values including str
, int
, float
, or for passing literal dict
s in places declared with a pydantic model class, I think that using a single cast()
could be an acceptable tradeoff for now.
[...] does it expose those type conversion routines so they can be invoked [...]
I understand that not currently. Maybe that could be a new feature request, and then these docs could be updated accordingly. But I think that with respect to this PR, these docs could be enough for now.
keeping editor support for BaseSetting's custom Config, but preventing __config__ from showing in constructor on editors supporting dataclass_transform
Quick note, I just updated the annotation for Before this last change: After the last change: And using |
…Var where suitable to improve editor support with type annotations and dataclass_transform
Is there anything left to do in this PR to add support or something we (pyright/pylance) need to handle? Hoping this can sneak its way into the next release. |
lancelot = Knight(title='Sir Lancelot', age=cast(Any, '23')) | ||
``` | ||
|
||
`cast(Any, '23')` doesn't affect the value, it's still just `'23'`, but now Pylance and mypy will assume it is of type `Any`, which means, they will act as if they didn't know the type of the value. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a small remark @tiangolo
Why not say cast(int, '23')
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the review, update to the PR, and the note @PrettyWood!
My intention with using cast(Any, '23')
was to make it something simple to copy-paste for other use cases and types.
For example, if it was something like a Dict[str, List[SomeModel]]
and the value was a dictionary with lists of dictionaries (instead of Pydantic models), e.g. parsed from some JSON, duplicating the annotation from the original field would be a bit cumbersome, would require duplication (which is always problematic and I think avoiding duplication is one of the greatest advantages of Pydantic), and adding the full type annotation/cast wouldn't provide much more help as it's only for mypy/Pylance.
Also, the idea was to make it explicit that this is just to "trick" mypy and Pylance to not pay attention to the actual types. And to avoid any possible confusion from users seeing a cast(int, '23')
, which is technically wrong (in terms of types).
And also to prevent them from thinking that they can just copy that anywhere else and assume it would work (e.g. thinking it would convert the types automatically or something). At least if they use cast(Any, '23')
in any other place, they lose autocompletion and can (hopefully) realize that the value is not an actual integer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense! Thanks 👍
…dantic#2721) * ✨ Add autocomplete support for VS Code, via dataclass_transform * 📝 Update changes * 📝 Add docs for VS Code * 📝 Clarify strict type error checks alternatives, include example with cast() * ♻️ Update BaseSettings annotation to improve editor support keeping editor support for BaseSetting's custom Config, but preventing __config__ from showing in constructor on editors supporting dataclass_transform * 🎨 Remove unused type: ignore comment * 🎨 Update type annotations for BaseSettings and BaseModel to use ClassVar where suitable to improve editor support with type annotations and dataclass_transform * 🎨 Apply ClassVars again * 📝 Simplify VS Code docs terms refer mainly to Pylance and clarify the relationship with Pyright * 📝 Add link to Pylance FAQ Co-authored-by: PrettyWood <em.jolibois@gmail.com>
Great job with this. As far as I could see this isn't released yet. Are there any plans for a release? Is there anything I can do to accelerate that? |
…dantic#2721) * ✨ Add autocomplete support for VS Code, via dataclass_transform * 📝 Update changes * 📝 Add docs for VS Code * 📝 Clarify strict type error checks alternatives, include example with cast() * ♻️ Update BaseSettings annotation to improve editor support keeping editor support for BaseSetting's custom Config, but preventing __config__ from showing in constructor on editors supporting dataclass_transform * 🎨 Remove unused type: ignore comment * 🎨 Update type annotations for BaseSettings and BaseModel to use ClassVar where suitable to improve editor support with type annotations and dataclass_transform * 🎨 Apply ClassVars again * 📝 Simplify VS Code docs terms refer mainly to Pylance and clarify the relationship with Pyright * 📝 Add link to Pylance FAQ Co-authored-by: PrettyWood <em.jolibois@gmail.com>
…dantic#2721) * ✨ Add autocomplete support for VS Code, via dataclass_transform * 📝 Update changes * 📝 Add docs for VS Code * 📝 Clarify strict type error checks alternatives, include example with cast() * ♻️ Update BaseSettings annotation to improve editor support keeping editor support for BaseSetting's custom Config, but preventing __config__ from showing in constructor on editors supporting dataclass_transform * 🎨 Remove unused type: ignore comment * 🎨 Update type annotations for BaseSettings and BaseModel to use ClassVar where suitable to improve editor support with type annotations and dataclass_transform * 🎨 Apply ClassVars again * 📝 Simplify VS Code docs terms refer mainly to Pylance and clarify the relationship with Pyright * 📝 Add link to Pylance FAQ Co-authored-by: PrettyWood <em.jolibois@gmail.com>
…dantic#2721) * ✨ Add autocomplete support for VS Code, via dataclass_transform * 📝 Update changes * 📝 Add docs for VS Code * 📝 Clarify strict type error checks alternatives, include example with cast() * ♻️ Update BaseSettings annotation to improve editor support keeping editor support for BaseSetting's custom Config, but preventing __config__ from showing in constructor on editors supporting dataclass_transform * 🎨 Remove unused type: ignore comment * 🎨 Update type annotations for BaseSettings and BaseModel to use ClassVar where suitable to improve editor support with type annotations and dataclass_transform * 🎨 Apply ClassVars again * 📝 Simplify VS Code docs terms refer mainly to Pylance and clarify the relationship with Pyright * 📝 Add link to Pylance FAQ Co-authored-by: PrettyWood <em.jolibois@gmail.com>
…dantic#2721) * ✨ Add autocomplete support for VS Code, via dataclass_transform * 📝 Update changes * 📝 Add docs for VS Code * 📝 Clarify strict type error checks alternatives, include example with cast() * ♻️ Update BaseSettings annotation to improve editor support keeping editor support for BaseSetting's custom Config, but preventing __config__ from showing in constructor on editors supporting dataclass_transform * 🎨 Remove unused type: ignore comment * 🎨 Update type annotations for BaseSettings and BaseModel to use ClassVar where suitable to improve editor support with type annotations and dataclass_transform * 🎨 Apply ClassVars again * 📝 Simplify VS Code docs terms refer mainly to Pylance and clarify the relationship with Pyright * 📝 Add link to Pylance FAQ Co-authored-by: PrettyWood <em.jolibois@gmail.com>
…dantic#2721) * ✨ Add autocomplete support for VS Code, via dataclass_transform * 📝 Update changes * 📝 Add docs for VS Code * 📝 Clarify strict type error checks alternatives, include example with cast() * ♻️ Update BaseSettings annotation to improve editor support keeping editor support for BaseSetting's custom Config, but preventing __config__ from showing in constructor on editors supporting dataclass_transform * 🎨 Remove unused type: ignore comment * 🎨 Update type annotations for BaseSettings and BaseModel to use ClassVar where suitable to improve editor support with type annotations and dataclass_transform * 🎨 Apply ClassVars again * 📝 Simplify VS Code docs terms refer mainly to Pylance and clarify the relationship with Pyright * 📝 Add link to Pylance FAQ Co-authored-by: PrettyWood <em.jolibois@gmail.com>
…dantic#2721) * ✨ Add autocomplete support for VS Code, via dataclass_transform * 📝 Update changes * 📝 Add docs for VS Code * 📝 Clarify strict type error checks alternatives, include example with cast() * ♻️ Update BaseSettings annotation to improve editor support keeping editor support for BaseSetting's custom Config, but preventing __config__ from showing in constructor on editors supporting dataclass_transform * 🎨 Remove unused type: ignore comment * 🎨 Update type annotations for BaseSettings and BaseModel to use ClassVar where suitable to improve editor support with type annotations and dataclass_transform * 🎨 Apply ClassVars again * 📝 Simplify VS Code docs terms refer mainly to Pylance and clarify the relationship with Pyright * 📝 Add link to Pylance FAQ Co-authored-by: PrettyWood <em.jolibois@gmail.com>
…dantic#2721) * ✨ Add autocomplete support for VS Code, via dataclass_transform * 📝 Update changes * 📝 Add docs for VS Code * 📝 Clarify strict type error checks alternatives, include example with cast() * ♻️ Update BaseSettings annotation to improve editor support keeping editor support for BaseSetting's custom Config, but preventing __config__ from showing in constructor on editors supporting dataclass_transform * 🎨 Remove unused type: ignore comment * 🎨 Update type annotations for BaseSettings and BaseModel to use ClassVar where suitable to improve editor support with type annotations and dataclass_transform * 🎨 Apply ClassVars again * 📝 Simplify VS Code docs terms refer mainly to Pylance and clarify the relationship with Pyright * 📝 Add link to Pylance FAQ Co-authored-by: PrettyWood <em.jolibois@gmail.com>
Is there any reason why this wasn't also applied to |
This is now the recommended way to set default values. Doing it the previous way causes pyright to assuming you're accidentally omitting required values. Doing it this way is somehow able to tell pyright that these fields are actually optional/have defaults and do not need to be specified. See [here][0] for details. Change seems to have been introduced by [this][1]. [0]: pydantic/pydantic#3753 (comment) [1]: pydantic/pydantic#2721
Change Summary
In short, this adds ✨ autocomplete ✨ for fields when creating a new instance of a Pydantic model. 🎉
Details
Eric Traut, Pyright's author, is writing a proposal for a new standard for Python
typing
to help declare models like pydantic and others as dataclass-like, to support autocompletion when creating a new instance (automatic typed__init__
constructor).The gist is that a new decorator
typing.dataclass_transform()
that would be applied in pydantic's metaclass (not needed by final users) would give it those extra powers.The draft standard is here: https://github.com/microsoft/pyright/blob/master/specs/dataclass_transforms.md
The discussion is here: microsoft/pyright#1782
The draft includes a small trick to adopt it right away even before the standard is accepted, adding a custom minimal
__dataclass_transform__
to the codebase. Pyright, and so, Pylance, and so VS Code, already support it.So, this simple change in this PR enables autocompletion in VS Code right away. In a similar way that it is provided in PyCharm by @koxudaxi's awesome plug-in.
As a side note, if this standard is accepted, I think it would also replace and/or simplify a large part of the pydantic mypy plugin.
Thanks @koxudaxi for pointing me to this in the discussions! #2698
Related issue number
I understand Samuel was already aware this was in progress and already in comm with Eric, so I'm not sure this justifies an issue. If so, let me know and I'll make one.
Checklist
changes/<pull request or issue id>-<github username>.md
file added describing change(see changes/README.md for details)