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

This is a prototype for #854 #855

Closed

Conversation

skinkie
Copy link
Contributor

@skinkie skinkie commented Oct 19, 2023

📒 Description

When a restriction is set, the ideal situation for the datamodel would be that only the properties that fall under the restriction are available. In the current situation, due to inheritance all attributes are available, while only some are valid.

Prototype for #854

🔗 What I've Done

I have added some code that creates 'null' fields when the inherited attributes are restricted. They are at rendering replaced with None.

💬 Comments

From https://www.reddit.com/r/learnpython/comments/12qhzi6/dataclasses_with_inheritance/ I found that kw_only was used to resolve the issue with non-default argument 'class name' follows default argument.

What seems to fail in this approach is that some inherited attributes somewhere get _attribute suffixed to the name. I am yet to find out why this happens.

🛫 Checklist

When a restriction is set, the ideal situation for the datamodel would be that only the properties that fall under the restriction are available.

From https://www.reddit.com/r/learnpython/comments/12qhzi6/dataclasses_with_inheritance/ I found that kw_only was used to resolve the issue with non-default argument 'class name' follows default argument.
@tefra
Copy link
Owner

tefra commented Oct 20, 2023

This approach I am afraid will work when we reach the EOL for python 3.9 and we can make the kw_only directive mandatory, until then we need to maintain compatibility.

In order to resolve some of these cases, we either remove the extension completely or mark the fields as prohibited.

Let' me take a look

@skinkie
Copy link
Contributor Author

skinkie commented Oct 20, 2023

This approach I am afraid will work when we reach the EOL for python 3.9 and we can make the kw_only directive mandatory, until then we need to maintain compatibility.

In order to resolve some of these cases, we either remove the extension completely or mark the fields as prohibited.

I can absolutely live with this functionality only to be applied when kwonly is used as argument.

Are you saying that prohibited would already give a better way? Hence my issue with suffixes "_attribute" vs "" would already been covered?

@skinkie
Copy link
Contributor Author

skinkie commented Oct 23, 2023

It seems that my interpretation which inhertitance to select is invalid. Hence... I have to do some more work.

@skinkie
Copy link
Contributor Author

skinkie commented Nov 5, 2023

@tefra I need a hint where to add RestrictedVar as module import so it is added only to the classes which actually use is.

Copy link

sonarqubecloud bot commented Nov 5, 2023

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 1 Code Smell

No Coverage information No Coverage information
0.0% 0.0% Duplication

@tefra
Copy link
Owner

tefra commented Nov 14, 2023

@tefra I need a hint where to add RestrictedVar as module import so it is added only to the classes which actually use is.

See the filters here, the alias might be tricky
https://github.com/tefra/xsdata/blob/main/xsdata/formats/dataclass/filters.py#L796

@skinkie
Copy link
Contributor Author

skinkie commented Dec 29, 2023

@tefra is this one something you also want to do 'correct' in "Tefra" style?

@tefra
Copy link
Owner

tefra commented Dec 29, 2023

@tefra is this one something you also want to do 'correct' in "Tefra" style?

It has to pass mypy :)

@tefra
Copy link
Owner

tefra commented Jan 2, 2024

Closing in favor of #908

@tefra tefra closed this Jan 2, 2024
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.

2 participants