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

All types tests #18

Closed
wants to merge 3 commits into from
Closed

Conversation

photonbit
Copy link
Contributor

Opening a separate PR to add a showcase for the tests in the new all_types file

@Relifest
Copy link
Collaborator

In order to address the issue of loop calls, such as the need for AI-EOTrainingData in the TrainingDataset, I chose to merge the original basic_types and extended-types into the current all_types. The original basic_types and extended-types are expected to be deprecated, and the current all_types can meet the functionality of the io module, such as tdml_deaders, tdmlw_writers, and yaml_to_tdml. Other functions need further modification.

@photonbit
Copy link
Contributor Author

Thank you for the response. The cyclic imports were operationally fixed in #14

From what I see in the type classes definition, it would be interesting to semantically group them, paying attention to the dependencies to avoid having cyclic imports. That way it would be easier to understand and maintain.

I see the changes that makes the io module deprecated, the work in my branch was following the same idea, although I have some doubts about the new methods introduced in #15 For example, I believe that most of the operations defined in thew new methods are already handled by pydantic, and that the functionality could be added just in the base class.

@Relifest
Copy link
Collaborator

Thank you very much for your suggestion. I am very sorry that my insufficient abilities have caused some problems with pyTDML.

I have noticed the modifications you provided in #14 . Previously, I hoped to fully follow the definitions in ISO, but if you feel it is necessary to semantically group them, I will follow the method in #14 .

Regarding what you said, most of the operations defined in the new method have already been processed by pydantic. Because the definition of ISO is nested layer by layer and many class parameters have multiple branches, I encountered an error when directly using pydantic to handle nested data structures before, so I chose this method. I will try to optimize it.

@photonbit photonbit closed this Oct 24, 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