-
-
Notifications
You must be signed in to change notification settings - Fork 2.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
Data Class Transforms add __dataclass_fields__
#15974
Comments
Where is The only reference to |
Yeah, you've got it. Nominally your approach is sound: if it's not in the spec, typeshed shouldn't build a protocol on it. Rather, #14263 should've been the way to go. The typeshed solution leverages a "plugin" behavior anyway (the existence of this undocumented attr) so might as well go "plugin" on this one.
|
FWIW, pyright also synthesizes this symbol, and it could be similarly argued that since it's an implementation detail of dataclasses, then pyright should not synthesize it, and then typeshed wouldn't "take advantage" of it (as they did in python/typeshed#9345). Or we can accept that we've grown to rely on it. 😔 This issue is about whether we should synthesize it indiscriminately for Data Class Transforms. Currently we do, and the following is a false positive: import typing
_T = typing.TypeVar("_T")
@typing.dataclass_transform()
def create_model(cls: type[_T]) -> type[_T]:
cls.__init__ = lambda *args, **kwargs: None # type: ignore
return cls
@create_model
class C:
a: int
c = C(a=42)
typing.reveal_type(c.__dataclass_fields__) Runtime:
mypy:
|
I don't think we should. PEP 681 is a specification for abstractions that work like dataclasses in their public interface; it doesn't require that abstractions that use |
@JelleZijlstra I agree, and this issue about removing this synthesized attribute in that case, basically -self._add_dataclass_fields_magic_attribute()
+if self._spec is _TRANSFORM_SPEC_FOR_DATACLASSES:
+ self._add_dataclass_fields_magic_attribute() At the same time, I did some sleuthing for a related issue, and it would appear that doing that would break many users that have grown to rely on |
I added a synthesized Here are some questions that may help inform the best way forward...
Thinking about potential solutions...
|
As mentioned here, pyserde, strawberry, ibis would use the parameter mentioned in option 2. In fact, so would flax I believe. |
PEP-681 says: "In the future, we may add additional parameters to dataclass_transform as needed to support common behaviors in user code. These additions will be made after reaching consensus on typing-sig rather than via additional PEPs." Should we start a topic in typing-sig on adding a |
Typing-sig is pretty much defunct at this point. I recommend using python/typing instead. But yes, that sounds like a good next step. I'm still not convinced that this is a problem that needs solving. It would be useful for you to present the motivation for a change in the discussion thread. |
Per PEP 681, data class transforms are defined in terms of dataclasses, but it doesn't mean they make dataclasses.
In particular, transformed classes don't necessarily have
__dataclass_fields__
or haveis_dataclass
return True, but per mypy - they do.The text was updated successfully, but these errors were encountered: