Pydantic 1.8.0 breaks use of custom __root__ in models

[butler54]

Describe the bug
Currently we use custom root models in situations where we have been unable to easily collapse the schema with fix_any.py

This tends to occur for us in situations where lists occur such as:

The result is a model such as this:

class LowerModel:
      __root__: str = Field(...,name='stuff')

class ATopLevelModel(OscalBaseModel):
      field: List[LowerModel]

Would result in an error when trying to create ATopLevelModel. See issue here:pydantic/pydantic#2449

It looks like this issue will be closed soon which will allow the removal of a hot fix.

However, a longer term question is why do we have the root models.

we should be able to fix this by removing our use of root. This would be a more appropriate long term solution.

To Reproduce
Force install pydantic==1.8.0
run make test
Observe errors

Expected behavior
No errors on make test.

[koxudaxi]

Hi, @butler54
I'm an author of datamodel-code-generator.
Thank you for using the code-generator in this project.
I have understood you use fix_any.py for fixing generated models.

Can I help you by fixing the code-generator?
I'm working on improving the code-generator.

However, a longer term question is why do we have the root models.

I guess LowerModel has some properties like name (usually maybe description or example )
So, the code-generator decide to need the __root__ model for the field.

I can implement an option to strip the un-required __root__ model on the code-generator.

Thank you

[fsuits]

Hi Koudia! I'm Frank Suits @fsuits and I work on the trestle project that relies on datamodel-code-generator and pydantic. Many thanks for your work on datamodel-code-generator! Yes, one of my roles has been to work on fix_any.py to solve some of the problems that come up when working with complex OSCAL schemas provided by NIST. Some months ago when I first worked on fix_any there were many issues I needed to address, but over time you have solved many of them and the OSCAL schemas have also simplified. But some problems remain. Today I updated fix_any.py and it is now greatly simplified. There are still some issues I need to handle and I will aim to send a follow up note shortly to summarize the remaining issues I deal with. It would be great if the __root__ classes could be absorbed into the classes that reference them because they serve no purpose as standalone classes. In the past few days there was a bug in pydantic 1.8.0 that was impacted by those __root__ classes, but that has been fixed in 1.8.1 today - so we are past that hurdle. But it would still be preferable not to have those classes. So thanks again for contacting and I will follow up with specific examples of the remaining issues that fix_any.py is dealing with. Best regards, Frank From: Koudai Aono <notifications@github.com> To: IBM/compliance-trestle <compliance-trestle@noreply.github.com> Cc: Frank Suits <frankst@au1.ibm.com>, Assign <assign@noreply.github.com> Date: 03/04/2021 05:55 AM Subject: [EXTERNAL] Re: [IBM/compliance-trestle] Pydantic 1.8.0 breaks use of custom __root__ in models (#381) Hi, @butler54 I'm an author of datamodel-code-generator. Thank you for using the code-generator in this project. I have understood you use fix_any.py for fixing generated models. Can I help you by fixing the code-generator? I'm working on improving Hi, @butler54 I'm an author of datamodel-code-generator. Thank you for using the code-generator in this project. I have understood you use fix_any.py for fixing generated models. Can I help you by fixing the code-generator? I'm working on improving the code-generator. However, a longer term question is why do we have the root models. I guess LowerModel has some properties like name (usually maybe description or example ) So, the code-generator decide to need the __root__ model for the field. I can implement an option to strip the un-required __root__ model on the code-generator. Thank you — You are receiving this because you were assigned. Reply to this email directly, view it on GitHub, or unsubscribe.

[koxudaxi]

@fsuits
Thank you for your response.

It would be great if the root classes could be absorbed into the
classes that reference them because they serve no purpose as standalone
classes.

I will add an option when I have the time.

There are still some issues I need to handle and I will aim to send a follow up note
shortly to summarize the remaining issues I deal with.

I look forward to the document.
Thank you for your cooperation.

Also, Would you please tell me how to reproduce your code generation?
I want to try it in my environment.

[fsuits]

Hi- I have documented the build process in: #389 It describes how to generate the models and what issues are fixed by running fix_any.py. Please let me know if you have questions. I'm not sure if the issue related to Part and = None is due to pydantic or datamodel-codegen - but I do need to make the change shown. Yes it would be great if the __root__ items could be absorbed into the classes, but it isn't a pressing issue right now. And regarding the need to generate separate classes like Location, Location1, Location2 - we will be in touch with NIST to see if the schemas can be changed, since using the same name in separate parts of the schema seems problematic. datamodel-codegen is doing a good thing by adding the 1, 2 etc. Thanks! Frank From: Koudai Aono <notifications@github.com> To: IBM/compliance-trestle <compliance-trestle@noreply.github.com> Cc: Frank Suits <frankst@au1.ibm.com>, Mention <mention@noreply.github.com> Date: 03/05/2021 05:06 AM Subject: [EXTERNAL] Re: [IBM/compliance-trestle] Pydantic 1.8.0 breaks use of custom __root__ in models (#381) @fsuits Thank you for your response. It would be great if the root classes could be absorbed into the classes that reference them because they serve no purpose as standalone classes. I will add an option when I have the time. ‍‍‍‍‍‍‍‍‍‍‍‍‍‍ @fsuits Thank you for your response. It would be great if the root classes could be absorbed into the classes that reference them because they serve no purpose as standalone classes. I will add an option when I have the time. There are still some issues I need to handle and I will aim to send a follow up note shortly to summarize the remaining issues I deal with. I look forward to the document. Thank you for your cooperation. Also, Would you please tell me how to reproduce your code generation? I want to try it in my environment. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

[koxudaxi]

Thank you for writing the document. I'm checking it.

I'm not sure if the issue related to Part and = None is due to pydantic or
datamodel-codegen - but I do need to make the change shown.

I deep-dived into pydantic source.
Pydantic blocks constraint conditions on self-reference class.
Because pydantic can't resolve infinite recursion.
Some day, the problem may be resolved.
But, the code-generator must not create an invalid model.
I will change that the code-generator ignores constraint conditions for the self-reference class.

• Certain items in lists have 'Item' appended to the name, which can be confusing because some items in the schema do in fact end with 'Item'. As a result the script removes 'Item' in the name except when it is expected.

I will add an option that doesn't add an Item to a singular object.

• Finally, in order to guarantee there are no induced forward references in the files, the classes are reordered to minimize the need for forwards, and any that can't be avoided are explicitly provided at the bottom of the file.

I think a good idea to sort the model. I will add an option for the feature too.

[fsuits]

Hi- Great! Many thanks for your interest in these topics. One thing I forgot to mention is that I sort the classes prior to output to make the final output as deterministic as possible - so that diffs are only generated as needed. So I load the classes, fix them, sort them, find dependencies, and re-order to minimize forward references. By starting with sorted classes and then shuffling to minimize forward references I help guarantee deterministic output. That is also the reason I don't want timestamp in the output. The separate tool I wrote to sort the classes is simply so I can compare the before and after of applying fix_any. When the classes are simply sorted the file is no longer usable - but it allows diffing to see what changes fix_any produced within the classes themselves. In that case I reorder both the 'before fix' files and the 'after fix' files - and diff them to see what was done by fix_any. Thanks! Frank From: Koudai Aono <notifications@github.com> To: IBM/compliance-trestle <compliance-trestle@noreply.github.com> Cc: Frank Suits <frankst@au1.ibm.com>, Mention <mention@noreply.github.com> Date: 03/06/2021 02:45 AM Subject: [EXTERNAL] Re: [IBM/compliance-trestle] Pydantic 1.8.0 breaks use of custom __root__ in models (#381) Thank you for writing the document. I'm checking it. I'm not sure if the issue related to Part and = None is due to pydantic or datamodel-codegen - but I do need to make the change shown. I deep-dived into pydantic source. Pydantic blocks constraint Thank you for writing the document. I'm checking it. I'm not sure if the issue related to Part and = None is due to pydantic or datamodel-codegen - but I do need to make the change shown. I deep-dived into pydantic source. Pydantic blocks constraint conditions on self-reference class. Because pydantic can't resolve infinite recursion. Some day, the problem may be resolved. But, the code-generator must not create an invalid model. I will change that the code-generator ignores constraint conditions for the self-reference class. Certain items in lists have 'Item' appended to the name, which can be confusing because some items in the schema do in fact end with 'Item'. As a result the script removes 'Item' in the name except when it is expected. I will add an option that doesn't add an Item to a singular object. Finally, in order to guarantee there are no induced forward references in the files, the classes are reordered to minimize the need for forwards, and any that can't be avoided are explicitly provided at the bottom of the file. I think a good idea to sort the model. I will add an option for the feature too. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

[koxudaxi]

@fsuits
Thank you for explaining the sorting.
I guess the feature is not required now. I don't implement it. because you have already the script.

I have released a new version 0.9.1.

I fixed the problem.

I will change that the code-generator ignores constraint conditions for the self-reference class.

And, I suggest two options '--disable-timestamp', '--disable-appending-item-suffix'
I tested these options with 0.9.1.
It works file.

Thank you.