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 Error when importing generated python data classes #324

Closed
kmgreg opened this issue Nov 23, 2020 · 9 comments
Closed

Type Error when importing generated python data classes #324

kmgreg opened this issue Nov 23, 2020 · 9 comments

Comments

@kmgreg
Copy link

kmgreg commented Nov 23, 2020

I am using Python 3.9.

Schema is: https://github.com/Vantiv/vantiv-sdk-for-python/blob/12.x/SchemaCombined_v12.17.xsd

Error is:
TypeError: typing.Optional requires a single type. Got Field(name=None,type=None,default=None,default_factory=<dataclasses._MISSING_TYPE object at 0x7ffff0.

Config for generate is:

<?xml version='1.0' encoding='UTF-8'?>
<Config xmlns="http://pypi.org/project/xsdata" version="20.11.1">
  <Output wsdl="false">
    <Package>test</Package>
    <Format>pydata</Format>
    <Structure>filenames</Structure>
    <CompoundFields>true</CompoundFields>
  </Output>
  <Conventions>
    <ClassName case="mixedCase" safePrefix="type"/>
    <FieldName case="mixedCase" safePrefix="value"/>
    <ModuleName case="mixedCase" safePrefix="mod"/>
    <PackageName case="mixedCase" safePrefix="pkg"/>
  </Conventions>
  <Aliases>
  </Aliases>
</Config>

I don't know if this is a peculiarity with the schema we use, or if there's some config option I'm missing.

@kmgreg
Copy link
Author

kmgreg commented Nov 23, 2020

So after looking at the generated file a bit, here is the troublesome line:

(Class RFRRequest)
accountUpdateFileRequestData: Optional[accountUpdateFileRequestData] = field(

@tefra
Copy link
Owner

tefra commented Nov 23, 2020

The field name matches the type name in the same context, that's a limitation of the language and the typing module. These conflicts will be fun to resolve 🙄

I understand the need to use mixedCase for class names eg RFRResponse -> Rfrresponse is ugly but for now
please stick to the default naming conventions, snakeCase for fields and pascalCase for classes and use aliases to bypass types names per case.

    <Aliases>
        <ClassName source="RFRResponse" target="RFRResponse" />

To be honest I never thought someone would use the same name case for class names and fields. I admit there is a missing layer to catch these conflicts but I also think it will be difficult to implement one.

Every class name needs to be compared against all field names and if there a conflict rename the field or the class, it sounds complicated. I am not dismissing the issue but I don't see an easy solution either.

@kmgreg
Copy link
Author

kmgreg commented Nov 23, 2020

Thanks for the quick reply! As you suggested, aliasing appears to have fixed what I wanted without the nasty side effects. Closing.

@kmgreg kmgreg closed this as completed Nov 23, 2020
@tefra
Copy link
Owner

tefra commented Nov 23, 2020

I am going to leave it open, I want to spend some time on this to either throw a user friendly exception during generation or go for a full blown solution.

@tefra tefra reopened this Nov 23, 2020
@kmgreg
Copy link
Author

kmgreg commented Dec 17, 2020

Although arguably less than ideal, one solution for this is to use the as command in the generated __init__.py file:

from vantivsdk.SchemaCombinedv1217 import (
	AccountInfoType as accountInfoType,
	AccountUpdate as accountUpdate,
	AccountUpdateFileRequestData as accountUpdateFileRequestData,
	AccountUpdateResponse as accountUpdateResponse,
        ...

@tefra
Copy link
Owner

tefra commented Dec 20, 2020

Aliases will work for imports but for local classes you will still have the same issue, the more I think about this the more I start to believe it's a mistake to allow the same naming scheme for both class and field names.

Just out of curiosity why do you want to have the same scheme for both them?

@kmgreg
Copy link
Author

kmgreg commented Dec 21, 2020

tl;dr: Needs to match:

  1. Old classes for backwards compatibility
  2. Schema for generating XML to send to server

PyXB is EOL and we need a substitute going forward to work with newer versions of Python.

My time as an intern for them is up, but they do need a replacement XSD to Python Class library and this seems like the best one that's still actively maintained.

@tefra
Copy link
Owner

tefra commented Dec 21, 2020

Oh you are trying to match 100% pyxb classes and field names. how does pyxb resolve this type of conflict?

Your best bet would be to use the default naming scheme, which is more pythonic in my opinion, and refactor the rest of the code. If I had more info and examples maybe we could figure out the naming scheme pyxb is using and try to replicate it.

@tefra
Copy link
Owner

tefra commented Jan 10, 2021

I suggest you switch to the MixedPacalName for classes
https://xsdata.readthedocs.io/en/latest/api/reference/xsdata.models.config.NameCase.html#xsdata.models.config.NameCase

At some point I will lock the codegen and raise an exception when someone is using the same scheme everywhere.

@tefra tefra closed this as completed Jan 10, 2021
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

No branches or pull requests

2 participants