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

fix!: python models not handling circular model dependencies #1946

Merged
merged 4 commits into from
Apr 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions docs/languages/Python.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

There are special use-cases that each language supports; this document pertains to **Python models**.

The generated Python models require at least v[3.7](https://docs.python.org/release/3.7.0/).

<!-- toc is generated with GitHub Actions do not remove toc markers -->

<!-- toc -->
Expand Down
8 changes: 7 additions & 1 deletion docs/migrations/version-3-to-4.md
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,8 @@ Console.WriteLine(dateTime2);

## Python

Models are aiming to be >= v3.7 compliant.

### Pydantic now follows v2 instead of v1

Reference: https://docs.pydantic.dev/2.6/migration/
Expand All @@ -104,7 +106,7 @@ class Message(BaseModel):
identifier: str = Field(description='''The Identifier for the Message''')
```

In Modelina 3 this used is rendered as:
In Modelina v3 this used is rendered as:

```python
class Message(BaseModel):
Expand Down Expand Up @@ -209,6 +211,10 @@ class Address:
return self._street_name
```

### Import style deprecation

All models are from this point onwards imported using explicit styles `from . import ${model.name}` to allow for circular model dependencies to work. This means that the option `importsStyle` is deprecated and is no longer in use. It will be removed at some point in the future.

## Go

### File names
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,9 @@

exports[`Should be able to render python models and should generate \`implicit\` or \`explicit\` imports for models implementing referenced types: nested-model 1`] = `
Array [
"
"from __future__ import annotations
from typing import Any, Dict

class ObjProperty:
def __init__(self, input: Dict):
if 'number' in input:
Expand All @@ -30,8 +31,9 @@ class ObjProperty:

exports[`Should be able to render python models and should generate \`implicit\` or \`explicit\` imports for models implementing referenced types: nested-model 2`] = `
Array [
"
"from __future__ import annotations
from typing import Any, Dict

class ObjProperty:
def __init__(self, input: Dict):
if 'number' in input:
Expand All @@ -58,14 +60,15 @@ class ObjProperty:

exports[`Should be able to render python models and should generate \`implicit\` or \`explicit\` imports for models implementing referenced types: root-model-explicit-import 1`] = `
Array [
"from ObjProperty import ObjProperty
"from __future__ import annotations
from typing import Any, Dict
from . import ObjProperty
class Root:
def __init__(self, input: Dict):
if 'email' in input:
self._email: str = input['email']
if 'obj_property' in input:
self._obj_property: ObjProperty = ObjProperty(input['obj_property'])
self._obj_property: ObjProperty.ObjProperty = ObjProperty.ObjProperty(input['obj_property'])

@property
def email(self) -> str:
Expand All @@ -75,25 +78,26 @@ class Root:
self._email = email

@property
def obj_property(self) -> ObjProperty:
def obj_property(self) -> ObjProperty.ObjProperty:
return self._obj_property
@obj_property.setter
def obj_property(self, obj_property: ObjProperty):
def obj_property(self, obj_property: ObjProperty.ObjProperty):
self._obj_property = obj_property
",
]
`;

exports[`Should be able to render python models and should generate \`implicit\` or \`explicit\` imports for models implementing referenced types: root-model-implicit-import 1`] = `
Array [
"from ObjProperty import ObjProperty
"from __future__ import annotations
from typing import Any, Dict
from . import ObjProperty
class Root:
def __init__(self, input: Dict):
if 'email' in input:
self._email: str = input['email']
if 'obj_property' in input:
self._obj_property: ObjProperty = ObjProperty(input['obj_property'])
self._obj_property: ObjProperty.ObjProperty = ObjProperty.ObjProperty(input['obj_property'])

@property
def email(self) -> str:
Expand All @@ -103,10 +107,10 @@ class Root:
self._email = email

@property
def obj_property(self) -> ObjProperty:
def obj_property(self) -> ObjProperty.ObjProperty:
return self._obj_property
@obj_property.setter
def obj_property(self, obj_property: ObjProperty):
def obj_property(self, obj_property: ObjProperty.ObjProperty):
self._obj_property = obj_property
",
]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ Array [
optional_field: Optional[str] = Field(description='''this field is optional''', default=None, serialization_alias='optionalField')
required_field: str = Field(description='''this field is required''', serialization_alias='requiredField')
no_description: Optional[str] = Field(default=None, serialization_alias='noDescription')
options: Optional[Options] = Field(default=None, serialization_alias='options')
options: Optional[Options.Options] = Field(default=None, serialization_alias='options')
",
]
`;
Expand Down
2 changes: 1 addition & 1 deletion src/generators/python/PythonConstrainer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ export const PythonDefaultTypeMapping: PythonTypeMapping = {
return constrainedModel.name;
},
Reference({ constrainedModel }): string {
return constrainedModel.name;
return `${constrainedModel.name}.${constrainedModel.name}`;
},
Any({ dependencyManager }): string {
dependencyManager.addDependency('from typing import Any');
Expand Down
42 changes: 32 additions & 10 deletions src/generators/python/PythonDependencyManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,7 @@ export class PythonDependencyManager extends AbstractDependencyManager {
* Simple helper function to render a dependency based on the module system that the user defines.
*/
renderDependency(model: ConstrainedMetaModel): string {
const useExplicitImports = this.options.importsStyle === 'explicit';
return `from ${useExplicitImports ? '.' : ''}${model.name} import ${
model.name
}`;
return `from . import ${model.name}`;
}

/**
Expand All @@ -26,14 +23,24 @@ export class PythonDependencyManager extends AbstractDependencyManager {
* For example `from typing import Dict` and `from typing import Any` would form a single import `from typing import Dict, Any`
*/
renderDependencies(): string[] {
let dependenciesToRender = this.dependencies;
dependenciesToRender =
this.mergeIndividualDependencies(dependenciesToRender);
dependenciesToRender = this.moveFutureDependency(dependenciesToRender);
return dependenciesToRender;
}

/**
* Split up each dependency that matches `from x import y` and keep everything else as is.
*
* Merge all `y` together and make sure they are unique and render the dependency as `from x import y1, y2, y3`
*/
private mergeIndividualDependencies(
individualDependencies: string[]
): string[] {
const importMap: Record<string, string[]> = {};
const dependenciesToRender = [];
/**
* Split up each dependency that matches `from x import y` and keep everything else as is.
*
* Merge all `y` together and make sure they are unique and render the dependency as `from x import y1, y2, y3`
*/
for (const dependency of this.dependencies) {
for (const dependency of individualDependencies) {
const regex = /from ([A-Za-z0-9]+) import ([A-Za-z0-9,\s]+)/g;
const matches = regex.exec(dependency);

Expand All @@ -58,4 +65,19 @@ export class PythonDependencyManager extends AbstractDependencyManager {
}
return dependenciesToRender;
}

/**
* If you import `from __future__ import x` it always has to be rendered first
*/
private moveFutureDependency(individualDependencies: string[]): string[] {
const futureDependencyIndex = individualDependencies.findIndex((element) =>
element.includes('__future__')
);
if (futureDependencyIndex !== -1) {
individualDependencies.unshift(
individualDependencies.splice(futureDependencyIndex, 1)[0]
);
}
return individualDependencies;
}
}
9 changes: 6 additions & 3 deletions src/generators/python/PythonGenerator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,9 @@ export interface PythonOptions
extends CommonGeneratorOptions<PythonPreset, PythonDependencyManager> {
typeMapping: TypeMapping<PythonOptions, PythonDependencyManager>;
constraints: Constraints<PythonOptions>;
/**
* @deprecated no longer in use - we had to switch to using explicit import style, always to support circular model dependencies.
*/
importsStyle: 'explicit' | 'implicit';
}
export type PythonConstantConstraint = ConstantConstraint<PythonOptions>;
Expand All @@ -62,7 +65,7 @@ export class PythonGenerator extends AbstractGenerator<
defaultPreset: PYTHON_DEFAULT_PRESET,
typeMapping: PythonDefaultTypeMapping,
constraints: PythonDefaultConstraints,
importsStyle: 'implicit',
importsStyle: 'explicit',
// Temporarily set
dependencyManager: () => {
return {} as PythonDependencyManager;
Expand Down Expand Up @@ -206,8 +209,8 @@ export class PythonGenerator extends AbstractGenerator<
.map((model) => {
return dependencyManagerToUse.renderDependency(model);
});
const outputContent = `${modelDependencies.join('\n')}
${outputModel.dependencies.join('\n')}
const outputContent = `${outputModel.dependencies.join('\n')}
${modelDependencies.join('\n')}
${outputModel.result}`;
return RenderOutput.toRenderOutput({
result: outputContent,
Expand Down
28 changes: 16 additions & 12 deletions src/generators/python/presets/Pydantic.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,30 +21,34 @@ const PYTHON_PYDANTIC_CLASS_PRESET: ClassPresetType<PythonOptions> = {
`class ${model.name}(BaseModel):`
);
},
property(params) {
let type = params.property.property.type;
const propertyName = params.property.propertyName;
property({ property, model, renderer }) {
let type = property.property.type;
const propertyName = property.propertyName;

if (params.property.property instanceof ConstrainedUnionModel) {
const unionTypes = params.property.property.union.map(
if (property.property instanceof ConstrainedUnionModel) {
const unionTypes = property.property.union.map(
(unionModel) => unionModel.type
);
type = `Union[${unionTypes.join(', ')}]`;
}

if (!params.property.required) {
if (!property.required) {
type = `Optional[${type}]`;
}
type = renderer.renderPropertyType({
modelType: model.type,
propertyType: type
});

const description = params.property.property.originalInput['description']
? `description='''${params.property.property.originalInput['description']}'''`
const description = property.property.originalInput['description']
? `description='''${property.property.originalInput['description']}'''`
: undefined;
const defaultValue = params.property.required ? undefined : 'default=None';
const jsonAlias = `serialization_alias='${params.property.unconstrainedPropertyName}'`;
const defaultValue = property.required ? undefined : 'default=None';
const jsonAlias = `serialization_alias='${property.unconstrainedPropertyName}'`;
let exclude = undefined;
if (
params.property.property instanceof ConstrainedDictionaryModel &&
params.property.property.serializationType === 'unwrap'
property.property instanceof ConstrainedDictionaryModel &&
property.property.serializationType === 'unwrap'
) {
exclude = 'exclude=True';
}
Expand Down
Loading
Loading