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

Expose DataclassInstance #115

Closed
NeilGirdhar opened this issue Feb 14, 2023 · 9 comments
Closed

Expose DataclassInstance #115

NeilGirdhar opened this issue Feb 14, 2023 · 9 comments

Comments

@NeilGirdhar
Copy link

Just wanted to track this so that I can update my code if it happens.

There are various times when it's necessary to annotate something as a dataclass including

  • functions that accept dataclasses only, and
  • and dataclass-makers that return subclasses of dataclass.

Therefore, it would be very convenient to eventually expose DataclassInstance. Suggested here python/typeshed#9362 (comment), and moved to _typeshed here: python/typeshed#9676.

@henryiii
Copy link

I'd like it exposed too, as with the latest mypy I'm getting things like:

error: Argument 1 to "fields" has incompatible type "object"; expected "Union[DataclassInstance, Type[DataclassInstance]]"  [arg-type]
error: Argument 1 to "fields" has incompatible type "Type[T]"; expected "Union[DataclassInstance, Type[DataclassInstance]]"  [arg-type]
error: Incompatible return value type (got "DataclassInstance", expected "T")

object to Any isn't bad, but not being able to bind T to DataclassInstance is painful!

For reference here from the linked issue, it seems there's currently some differences in how this is handled between type checkers (pyright specifically), which is why it's not exposed yet.

@AlexWaygood
Copy link
Member

AlexWaygood commented Mar 14, 2023

but not being able to bind T to DataclassInstance is painful!

There's always the good-old TYPE_CHECKING hack (though I appreciate it's ugly):

from typing import TypeVar, TYPE_CHECKING

if TYPE_CHECKING:
    from _typeshed import DataclassInstance

DataclassT = TypeVar("DataclassT", bound="DataclassInstance")

For reference here from the linked issue, it seems there's currently some differences in how this is handled between type checkers (pyright specifically), which is why it's not exposed yet.

Also it's still pretty new, and new things in the _typeshed directory tend to change and/or get removed if we change our minds or think of something better. So it can't really be considered stable yet.

@JelleZijlstra
Copy link
Member

Our general principle is to only add things that are in typing or in an active PEP. So if you want this, first convince us to add it to typing for Python 3.12, then we'll also add it here.

@NeilGirdhar
Copy link
Author

first convince us to add it to typing for

One solution would be for Dataclass to be a class exposed in dataclasses that

  • supports instance and subclass checks using the same logic as dataclasses.is_dataclass,
  • has an attribute __dataclass_fields__, and
  • is a dataclass-maker by calling dataclasses.dataclass in __init_subclass__.

What do you think?

@JelleZijlstra
Copy link
Member

That would mean people can write class Foo(Dataclass): ..., right? Not sure if I like that; it goes against "there is only one way to do it". It would certainly be a much harder sell than a typing-only protocol.

@AlexWaygood
Copy link
Member

AlexWaygood commented Mar 14, 2023

I agree with Jelle -- that sounds like an odd approach to me. I would just do the same thing as we did for os.PathLike. Just add an ABC to dataclasses.py (with a custom __subclasshook__ method) at runtime, which in the stubs we say is a runtime-checkable protocol. That way it's in the right module (dataclasses, not typing), but dataclasses.py doesn't have its import time slowed down by having to import typing.

Anyway, that whole discussion seems pretty off-topic for this issue tracker, and should probably be discussed over at python/typing (or the CPython issue tracker) rather than here.

@NeilGirdhar
Copy link
Author

NeilGirdhar commented Mar 14, 2023

right? Not sure if I like that; it goes against "there is only one way to do it".

Yes, but that's already baked into the dataclass-maker standard (typing.dataclass_transform). A dataclass-maker is either a decorator or a class that you can inherit from.

Just add an ABC

Right, that's basically what I'm suggesting, but once you have the ABC, you may as well make directly inheriting from it work as well, no? Also, you don't really need toe abc.ABC registration machinery.

Anyway, that whole discussion seems pretty off-topic for this issue tracker, and should probably be discussed over at python/typing (or the CPython issue tracker) rather than here

Sure, where would you like me to post it?

@AlexWaygood
Copy link
Member

AlexWaygood commented Mar 14, 2023

Right, that's basically what I'm suggesting, but once you have the ABC, you may as well make directly inheriting from it work as well, no?

I'd probably suggest adding an __init_subclass__ to make it unsubclassable tbh. It's a conscious design decision that dataclasses uses decorator-based semantics rather than inheritance-based semantics. I agree with Jelle that it would be pretty confusing to make it possible to inherit from a dataclasses.DataclassInstance class and have this one thing in the module that you can inherit from, whereas everything else works via decorators.

Sure, where would you like me to post it?

CPython works I guess, since the concrete proposal is to add something to the standard library.

@JelleZijlstra
Copy link
Member

Closing this following the policy to only include things that are in CPython or in a PEP. Let's get it into CPython first.

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

No branches or pull requests

4 participants