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

Remove models with same set of field. #170

Closed
gk-patel opened this issue Jul 8, 2020 · 6 comments
Closed

Remove models with same set of field. #170

gk-patel opened this issue Jul 8, 2020 · 6 comments
Labels
documentation Improvements or additions to documentation enhancement New feature or request released

Comments

@gk-patel
Copy link

gk-patel commented Jul 8, 2020

It would be good if we can identify that a similar model exists and use that model instead of creating a new model, especially when the FIELDS and DATA-TYPE are same.

For example, when my data is as follows,

{
    "Arm Right": {
        "Joint 1": 5,
        "Joint 2": 3, 
        "Joint 3": 66
    }, 
    "Arm Left": {
        "Joint 1": 55,
        "Joint 2": 13, 
        "Joint 3": 6
    }
}

we get the following output,

class ArmRight(BaseModel):
    Joint_1: int = Field(..., alias='Joint 1')
    Joint_2: int = Field(..., alias='Joint 2')
    Joint_3: int = Field(..., alias='Joint 3')

class ArmLeft(BaseModel):
    Joint_1: int = Field(..., alias='Joint 1')
    Joint_2: int = Field(..., alias='Joint 2')
    Joint_3: int = Field(..., alias='Joint 3')

class Model(BaseModel):
    Arm_Right: ArmRight = Field(..., alias='Arm Right')
    Arm_Left: ArmLeft = Field(..., alias='Arm Left')

Describe the solution you'd like
I think it would be good if we can detect that models for ArmLeft and ArmRight are the same, and then to just use one of these models. For example,

class ArmRight (BaseModel):
    Joint_1: int = Field(..., alias='Joint 1')
    Joint_2: int = Field(..., alias='Joint 2')
    Joint_3: int = Field(..., alias='Joint 3')

class Model(BaseModel):
    Arm_Right: ArmRight = Field(..., alias='Arm Right')
    Arm_Left: ArmRight = Field(..., alias='Arm Left')

Describe alternatives you've considered
Currently, I just delete manually. But, we have a big system with many similar structures in JSON and many time we need to manually delete about 1000 models. :(

Thank you very much for this excellent tool.

@koxudaxi
Copy link
Owner

koxudaxi commented Jul 9, 2020

@gk-patel
Thank you for creating this issue.
I think it's a good idea.
The behavior may have to be an option of the command line 🤔

I will implement this feature after remove bugs.

@koxudaxi koxudaxi added the enhancement New feature or request label Jul 23, 2020
@koxudaxi
Copy link
Owner

@gk-patel
I have released a new version as 0.6.15.
The version has the --reuse-model option that you want.

I put an example.

Input

{
"Arm Right": {
"Joint 1": 5,
"Joint 2": 3,
"Joint 3": 66
},
"Arm Left": {
"Joint 1": 55,
"Joint 2": 13,
"Joint 3": 6
},
"Head": {
"Joint 1": 10
}
}

Output

class ArmRight(BaseModel):
Joint_1: int = Field(..., alias='Joint 1')
Joint_2: int = Field(..., alias='Joint 2')
Joint_3: int = Field(..., alias='Joint 3')
class ArmLeft(BaseModel):
Joint_1: int = Field(..., alias='Joint 1')
Joint_2: int = Field(..., alias='Joint 2')
Joint_3: int = Field(..., alias='Joint 3')
class Head(BaseModel):
Joint_1: int = Field(..., alias='Joint 1')
class Model(BaseModel):
Arm_Right: ArmRight = Field(..., alias='Arm Right')
Arm_Left: ArmRight = Field(..., alias='Arm Left')
Head: Head

I'm sorry, My work is too late.

@gk-patel
Copy link
Author

gk-patel commented Dec 30, 2020

@koxudaxi No worries man, looks nice.

Although, the following would have been much more cooler,

...
class ArmLeft(ArmRight): 
   pass 
...

If you can do that, it would look much nicer. But if you cannot, then you can close the issue. I leave it up to you.

@koxudaxi
Copy link
Owner

@gk-patel
No problem, It's a good idea.
I can implement the changes easily.

But, I found a problem with a case.
The code generator generates the Enum class from JSONSchema/OpenAPI.
Enum class can't be extended.

>> class B(A): pass
TypeError: Cannot extend enumerations

We should assign to another value. or we don't touch duplicated enum 🤔

@gk-patel
Copy link
Author

gk-patel commented Jan 1, 2021

Well, IMHO the easiest way would be to check if the content of enums is exactly the same,

  • if it is exactly the same, just use the same name everywhere (so in that case, dont create class B)
  • if it not exactly the same, the create a seperate enum without inheritance, (so in that case, create class A and B)

Do you have any arguments against this idea ?

@koxudaxi
Copy link
Owner

koxudaxi commented Jan 7, 2021

@gk-patel
OK. I released a new version as 0.6.17
This version has changed your idea.

class ArmLeft(ArmRight):
pass
class Head(BaseModel):
Joint_1: int = Field(..., alias='Joint 1')
class Model(BaseModel):
Arm_Right: ArmRight = Field(..., alias='Arm Right')
Arm_Left: ArmLeft = Field(..., alias='Arm Left')

The enum will reuse the same model. A duplicate model will be created too. Because some models may need the enum from other modules.

class Animal(Enum):
dog = 'dog'
cat = 'cat'
snake = 'snake'
class Pet(Enum):
dog = 'dog'
cat = 'cat'
snake = 'snake'
class User(BaseModel):
name: Optional[str] = None
animal: Optional[Animal] = None
pet: Optional[Animal] = None

@koxudaxi koxudaxi added documentation Improvements or additions to documentation released labels Jan 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request released
Projects
None yet
Development

No branches or pull requests

2 participants