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

Adding support for dataclass Union field #63

Closed
panchock opened this issue Oct 26, 2022 · 8 comments
Closed

Adding support for dataclass Union field #63

panchock opened this issue Oct 26, 2022 · 8 comments

Comments

@panchock
Copy link

Hi there!

We are starting using this package a lot in our product and it's great!
One gap we reached into is support for dataclass union (typing.Union[dataclass1, dataclass2, ...]) as one of the dataclass fields.

Example case:

@dataclass
class A:
    a: int


@dataclass
class B:
    b: int


@dataclass
class Response:
    obj: A | B

class ResponseSerializer(DataclassSerializer)
    class Meta:
        dataclass = Response

I solved this by extending DataclassSerializer and add support using rest-polymorphic.
We also use drf-spectacular for openapi scheme and it also supports rest-polymorphic, so all good!

Before I open PR I wanted to check that this feature is useful, I would be happy to get a feedback :)

@oxan
Copy link
Owner

oxan commented Oct 26, 2022

I'm happy to consider a PR to add support for unions, provided that it doesn't add a hard dependency (i.e. if you don't use unions, you shouldn't need the dependency).

@panchock
Copy link
Author

Happy to hear it will be useful.
My implementation based on polymorphic serializer so it will require rest-polymorphic packege as requirement.
I'll open the PR and we will discuss there.

@rooterkyberian
Copy link

@panchock care to share your solution? a "dirty" DataclassSerializer modification (shared through gist or something) will be nice as well if you don't have a time for a proper PR.

@johnthagen
Copy link

johnthagen commented Apr 13, 2023

This would be a really great feature, especially given that it would plug into drf-spectacular.

In order to avoid concerns about other users being required to add a new dependencies, django-rest-polymorphic could be added as a optional dependency:

Something like:

[project.optional-dependencies]
union = ["django-rest-polymorphic"]

And then users could opt-in to this feature/dependency:

pip install djangorestframework-dataclasses[union]

@johnthagen
Copy link

I'll open the PR and we will discuss there.

@panchock Were you ever able to open a PR?

@panchock
Copy link
Author

I'm planning to open PR soon.
@oxan Do I need any permissions to push my branch?

@johnthagen
Copy link

@panchock You don't need permissions to open a PR. You will create your own local fork, create a branch there, and then GitHub will help you set up the pull request back to this original repository.

@oxan
Copy link
Owner

oxan commented Aug 21, 2023

Fixed by #82.

@oxan oxan closed this as completed Aug 21, 2023
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 a pull request may close this issue.

4 participants