Skip to content

Type hints for compound fields#858

Merged
tefra merged 2 commits intotefra:mainfrom
zemanj:compound_fields_type_hints
Nov 4, 2023
Merged

Type hints for compound fields#858
tefra merged 2 commits intotefra:mainfrom
zemanj:compound_fields_type_hints

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
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