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

Issues 1454 typeddict not required optional bug fix #1457

Conversation

kylebebak
Copy link
Contributor

@kylebebak kylebebak commented Jul 27, 2023

  • Fixes DCG incorrectly marks non-nullable, not required TypedDict keys as NotRequired[Optional] #1454, see this issue for more details
  • Makes changes to model.typed_dict.DataModelField, and DataModelFieldBase, that only affect how DCG renders TypedDict output
    • With TypedDict, don't force property to be nullable (Optional) just because it's NotRequired
  • Adds tests, see especially tests/data/expected/main/main_typed_dict_not_required_nullable/output.py and tests/data/jsonschema/not_required_nullable.json
  • Also worth mentioning, for Open API 3 schemas I think the TypedDict output is still not ideal
    • I couldn't get an output TypedDict to have both NotRequired[Optional] even when a property is explicitly marked nullable and not marked required
    • Maybe this is an Open API 3.0 limitation, but not JSON Schema limitation? Or an edge case bug in the Open API parser?

@codecov
Copy link

codecov bot commented Aug 1, 2023

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (e9b6edf) 100.00% compared to head (2e82949) 100.00%.
Report is 685 commits behind head on master.

Additional details and impacted files
@@             Coverage Diff             @@
##            master     #1457     +/-   ##
===========================================
  Coverage   100.00%   100.00%             
===========================================
  Files           11        32     +21     
  Lines         1020      3565   +2545     
  Branches       201       837    +636     
===========================================
+ Hits          1020      3565   +2545     
Flag Coverage Δ
unittests 99.63% <99.58%> (?)

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

Files Changed Coverage Δ
datamodel_code_generator/__init__.py 100.00% <100.00%> (ø)
datamodel_code_generator/__main__.py 100.00% <100.00%> (ø)
datamodel_code_generator/format.py 100.00% <100.00%> (ø)
datamodel_code_generator/http.py 100.00% <100.00%> (ø)
datamodel_code_generator/imports.py 100.00% <100.00%> (ø)
datamodel_code_generator/model/__init__.py 100.00% <100.00%> (ø)
datamodel_code_generator/model/base.py 100.00% <100.00%> (ø)
datamodel_code_generator/model/dataclass.py 100.00% <100.00%> (ø)
datamodel_code_generator/model/enum.py 100.00% <100.00%> (ø)
datamodel_code_generator/model/imports.py 100.00% <100.00%> (ø)
... and 22 more

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

@koxudaxi
Copy link
Owner

koxudaxi commented Aug 2, 2023

Thank you for creating the PR.

I couldn't get an output TypedDict to have both NotRequired[Optional] even when a property is explicitly marked nullable and not marked required

You should use --strict-nullable option for the behavior.
https://github.com/kylebebak/datamodel-code-generator/blob/37969cecea05e8d41b8a7427b108dcb7c831a441/tests/data/expected/main/main_typed_dict_nullable/output.py#L10-L14

@koxudaxi koxudaxi merged commit 0deef65 into koxudaxi:master Aug 3, 2023
73 checks passed
@kylebebak
Copy link
Contributor Author

Thanks for reviewing, cleaning this up, and getting this merged @koxudaxi !

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.

DCG incorrectly marks non-nullable, not required TypedDict keys as NotRequired[Optional]
2 participants