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

Type hints for compound fields #858

Merged
merged 2 commits into from
Nov 4, 2023
Merged

Conversation

zemanj
Copy link
Contributor

@zemanj zemanj commented Oct 24, 2023

📒 Description

Compound fields are no longer type-hinted as list[object].
Instead, the type hints of compound fields now reflect the types in the corresponding <xsd:choice> definition.

Resolves #857

🔗 What I've Done

  • Changed the compound field handler in xsdata/codegen/handlers/create_compound_fields.py such that it passes a de-duplicated list comprising the types of the combined choices as type hints.
  • Adjusted the expected attribute in tests/codegen/handlers/test_create_compound_fields.pyaccordingly.
  • Changed the resulting model in tests/fixtures/compound/models.pyaccordingly.
  • To not break the XmlParser, I had to reorder the logic in XmlVar.is_element/XmlVar.is_elements determination such that XmlType.ELEMENTS is picked up before XmlType.ELEMENT.
  • I changed XmlVarBuilder.is_valid() such that object is allowed to be listed along with other types in a Union for XML type XmlType.ELEMENTS.
    This is useful to get proper type hinting when working with xsdata-generated code in an IDE.

💬 Comments

Are there additional tests needed explicitly covering this feature?

🛫 Checklist

@codecov
Copy link

codecov bot commented Oct 24, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (39f7254) 99.89% compared to head (de68b2e) 99.89%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #858      +/-   ##
==========================================
- Coverage   99.89%   99.89%   -0.01%     
==========================================
  Files         104      104              
  Lines        9296     9295       -1     
  Branches     2077     2078       +1     
==========================================
- Hits         9286     9285       -1     
  Misses          3        3              
  Partials        7        7              
Files Coverage Δ
xsdata/codegen/handlers/create_compound_fields.py 100.00% <100.00%> (ø)
xsdata/formats/dataclass/models/builders.py 100.00% <100.00%> (ø)
xsdata/formats/dataclass/models/elements.py 100.00% <100.00%> (ø)

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

@zemanj zemanj force-pushed the compound_fields_type_hints branch from b04f946 to 653bec5 Compare October 24, 2023 00:16
@zemanj zemanj force-pushed the compound_fields_type_hints branch from 653bec5 to 2772c6c Compare October 24, 2023 00:24
@zemanj
Copy link
Contributor Author

zemanj commented Oct 24, 2023

The change breaks several W3C tests (mostly "XmlContextError: Xml type 'Elements' does not support typing: ...").
I can take a look later this week.

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@zemanj
Copy link
Contributor Author

zemanj commented Oct 28, 2023

W3C tests pass now. Not sure what to do with sample test as these are not included in the CI of this package and seem to be broken at the moment (since this commit).

Copy link
Owner

@tefra tefra left a comment

Choose a reason for hiding this comment

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

This is brilliant!!!

Obviously you had to dig very deep, I was amazed how simple the solution was, I always thought I would have to do a lot of refactoring to support this. The samples repo 🤦 was updated to reflect some changes from another feature pr, sorry about that I checked it manually and everything works as expected!

Thank you very much for this contribution

@tefra tefra merged commit 075e9da into tefra:main Nov 4, 2023
14 checks passed
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.

Type hints for compound fields
2 participants