Python Generator now creates types = None in classes for all optional types #1165
Conversation
… types This was previously breaking literal? types
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
There was a problem hiding this comment.
❌ Changes requested. Reviewed everything up to e467f07 in 53 seconds
More details
- Looked at
579lines of code in3files - Skipped
0files when reviewing. - Skipped posting
0drafted comments based on config settings.
Workflow ID: wflow_aAbzK8jPiYiviPWn
Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
| @@ -160,11 +160,11 @@ impl<'ir> From<ClassWalker<'ir>> for PartialPythonClass<'ir> { | |||
| } | |||
| } | |||
|
|
|||
| pub fn add_default_value(node: &FieldType, type_str: &String) -> String { | |||
| if type_str.starts_with("Optional[") { | |||
| pub fn add_default_value(node: &FieldType, type_str: String) -> String { | |||
There was a problem hiding this comment.
The change from &String to String in the add_default_value function signature is unnecessary and can lead to unnecessary cloning. Consider reverting to &String for efficiency.
| pub fn add_default_value(node: &FieldType, type_str: String) -> String { | |
| pub fn add_default_value(node: &FieldType, type_str: &String) -> String { |
|
note for the future, we cant merge this since we need default None types or anyone using these models with fastapi or whatnot will get exceptions (fastapi requires default None types for all optionals) |
This was previously breaking literal? types
Important
The PR updates the Python generator to remove default
Noneassignments for optional fields, fixing issues with literal types.add_default_valueingenerate_types.rsto remove defaultNonefor optional types.Nonevalues.generate_types.rs: Modifiedadd_default_valuefunction.partial_types.py: Removed defaultNonefrom optional fields in classes likeBigNumbers,BinaryNode, andBlockConstraint.types.py: Removed defaultNonefrom optional fields in classes likeContactInfo,CustomTaskResult, andNested.Noneassignments.This description was created by
for e467f07. It will automatically update as commits are pushed.