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

dataclass generator improvements #2102

Merged
merged 7 commits into from
Oct 16, 2024

Conversation

anis-campos
Copy link
Contributor

@anis-campos anis-campos commented Oct 11, 2024

Use apply_discriminator_type for dataclasses
Allow dataclass models to be properly generated with discriminator field

Fix dataclass inheritance
Thanks to keyword only, dataclass models can use inheritance and no have issues with default values

Support datetime types in dataclass fields

applying --output-datetime-class from #2100 to dataclass to map date, time and date time to the python datetime objects instead of strings.

fixes #1966

Copy link

codspeed-hq bot commented Oct 11, 2024

CodSpeed Performance Report

Merging #2102 will not alter performance

Comparing anis-campos:dataclass-generator-improvements (8cd56d4) with main (2df133c)

Summary

✅ 31 untouched benchmarks

@anis-campos anis-campos force-pushed the dataclass-generator-improvements branch from 88014ad to 19debe8 Compare October 11, 2024 07:05
Copy link

codecov bot commented Oct 11, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (2df133c) to head (8cd56d4).
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff            @@
##              main     #2102   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           38        38           
  Lines         4222      4250   +28     
  Branches       979       984    +5     
=========================================
+ Hits          4222      4250   +28     
Flag Coverage Δ
unittests 99.67% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@koxudaxi
Copy link
Owner

@anis-campos
Thank you for submitting this pull request. I have two points that need to be addressed:

Since this change alters both the generated code and its behavior, could you please implement the kw_only feature as a new CLI option?
The kw_only parameter is a feature introduced in Python 3.10. We need to ensure that the output version is compatible with this feature. Please refer to the official documentation for more details:
https://docs.python.org/3/library/dataclasses.html#dataclasses.KW_ONLY

I would appreciate it if you could address these points. Thank you for your contribution, and I look forward to reviewing the updated version.

Copy link
Owner

@koxudaxi koxudaxi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left the comment.

@anis-campos anis-campos force-pushed the dataclass-generator-improvements branch 2 times, most recently from e81eefa to c29da70 Compare October 14, 2024 08:53
@anis-campos
Copy link
Contributor Author

@koxudaxi I added a cli flag, let me know if the naming is right and if I could have changed less signatures with a different strategy to pass the info down to the model template.

@anis-campos
Copy link
Contributor Author

@koxudaxi I'm also adding the support for datetime in dataclass, I'm using the recently merged --output-datetime-class to do so

@anis-campos anis-campos force-pushed the dataclass-generator-improvements branch from 783fb89 to 2e07b26 Compare October 14, 2024 10:32
Anis Da Silva Campos added 2 commits October 15, 2024 09:53
Allow dataclass models to be properly generated with discriminator field
Thanks to keyword only, dataclass models can use inheritance and no have issues with default values
@anis-campos anis-campos force-pushed the dataclass-generator-improvements branch from 2e07b26 to 9984613 Compare October 15, 2024 08:24
@anis-campos anis-campos requested a review from koxudaxi October 15, 2024 08:24
@anis-campos anis-campos force-pushed the dataclass-generator-improvements branch from 9984613 to 16fa487 Compare October 15, 2024 08:26
applying `--output-datetime-class` from koxudaxi#2100 to dataclass to map date, time and date time to the python `datetime` objects instead of strings.
@anis-campos anis-campos force-pushed the dataclass-generator-improvements branch from 16fa487 to 4d67a81 Compare October 15, 2024 08:26
@meliache
Copy link
Contributor

I think this might fix also #2002 where --use-one-literal-as-default didn't work for discriminators. At least in my organizations codebase I thought that #2002 is the problem and this PR-branch seems to fix it. Thanks a lot, looking forward very much to see this merged and eventually in a release!

@anis-campos
Copy link
Contributor Author

anis-campos commented Oct 15, 2024

I think this might fix also #2002 where --use-one-literal-as-default didn't work for discriminators. At least in my organizations codebase I thought that #2002 is the problem and this PR-branch seems to fix it. Thanks a lot, looking forward very much to see this merged and eventually in a release!

thanks @meliache , I appreciate your comment.

I don't think it is linked, the issues is referencing pyantic output, I'm reworking only the dataclass generator.

side note, I would close the issue you linked, executing the example I see no issue with the pydantic output I have the default values just fine

# generated by datamodel-codegen:
#   filename:  openapi.yaml
#   timestamp: 2024-10-15T09:02:19+00:00

from __future__ import annotations

from typing import Literal, Union

from pydantic import BaseModel, Field, RootModel


class StartEvent(BaseModel):
    type: Literal['start'] = 'start'


class EndEvent(BaseModel):
    type: Literal['end'] = 'end'


class ResponseEvent(RootModel[Union[StartEvent, EndEvent]]):
    root: Union[StartEvent, EndEvent] = Field(..., discriminator='type')

koxudaxi and others added 4 commits October 16, 2024 01:51
for more information, see https://pre-commit.ci
@meliache
Copy link
Contributor

@anis-campos

I don't think it is linked, the issues is referencing pyantic output, I'm reworking only the dataclass generator.

You are right, the linked issue seems to be already resolved in v0.26.1. In my project I had been using v0.25.9 and I had seen the same issue. In the release notes to v0.26.0 and v0.26.1 I saw nothing that indicated that explicitly indicated that the issue might be fixed. In this PR you at least mentioned that you worked on discriminator support, so on a whim I decided to try this branch, but then I tried v0.26.1 and there it also worked already.

@koxudaxi koxudaxi merged commit d0c0f16 into koxudaxi:main Oct 16, 2024
76 checks passed
@anis-campos anis-campos deleted the dataclass-generator-improvements branch October 16, 2024 16:19
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.

Dataclass code gen error: TypeError: non-default argument 'XYZ' follows default argument
3 participants