-
Notifications
You must be signed in to change notification settings - Fork 0
Feature/maxim start #1
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
base: main
Are you sure you want to change the base?
Conversation
JosePizarro3
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some work to do
JosePizarro3
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few more comments, more importantly, the implementation in jflat.py needs some changes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why do we need an AI generated json?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, DISCUSSION:
I was wondering, if the flatten Output is good.
Copilote generated a json schema, maybe there is room for improvement?
|
|
||
|
|
||
| class Person(BaseModel): | ||
| name: str = Field(..., description="The person's name") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe you can define more types in this example? Something like str | none, and create more different Fields (so the printed JSON schema is actually richer and we can cover more cases)
| from typing import Any, Dict | ||
|
|
||
|
|
||
| def flatten_json(data: Dict[str, Any], parent_key: str = "") -> Dict[str, Any]: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The goal of this function is more to map a JSON Schema (as printed by pydantic) to our new flattened version. The end result should look something like:
{
"$defs": {
"Method": {
"properties": {
"author": {
"description": "The author of the method",
"type": "string"
},
"method_name": {
"description": "The name of the method",
"type": "string"
},
"person": {
"$ref": "#/$defs/Person"
}
},
"description": "...<whatever-here>...",
"$inherits_from": "#/$defs/BaseMethod",
},
<other classes here>
}}As you can see, I slightly modified the resulting JSON schema when printed using model_json_schema. The idea is to get rid off unnecessary stuff and adding some other info. I:
- Deleted
titlein each property - Deleted
titlein each object - Added an
$inherits_fromkey in each of the objects dictionaries - Moved the Method defs inside
$defs(before, it is outside because we are printing from it) - We need to add
BaseMethodto define inheritances - Deleted all the
requiredandtype:objectstuff
We could also:
- Add a key inside each property defining if they are mandatory or optional (this was before defined by
required. Somethind like:
"author": {
"description": "The author of the method",
"type": "string",
"mandatory": true # this can also be false, if the property is optional (i.e., str | None)
},
No description provided.