Skip to content

mps-cli-py: extended SModel class to include imported models#63

Merged
danielratiu merged 2 commits into
mbeddr:mainfrom
btakim:main
Jun 17, 2026
Merged

mps-cli-py: extended SModel class to include imported models#63
danielratiu merged 2 commits into
mbeddr:mainfrom
btakim:main

Conversation

@btakim

@btakim btakim commented Jun 17, 2026

Copy link
Copy Markdown
Collaborator

There is a need to analyze imported models when the model is read to be able to get the right model dependency graph. Added tests for model imports.

@btakim btakim changed the title python.cli: extended SModel class for include imported models mps-cli-py: extended SModel class for include imported models Jun 17, 2026
@btakim btakim changed the title mps-cli-py: extended SModel class for include imported models mps-cli-py: extended SModel class to include imported models Jun 17, 2026

@Prithvi686 Prithvi686 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @btakim ,

The changes look good to me! Thanks for this extension, however I just have one small comment here - extract_imported_models() parses the same XML twice for every model I think - once in extract_model_core_info for the new imported_models field and again in extract_imports_and_registry for index_2_imported_model_uuid.

Not wrongg, just doing the same work twice. Could we pass the dict through instead of recomputing it? But I think we can also leave this as a follow-up and fix it later...

@btakim

btakim commented Jun 17, 2026

Copy link
Copy Markdown
Collaborator Author

@Prithvi686 thanks for catching the issue, seperated registry extraction from model import extraction now. I think registry can also be part of the SModel later but for now that looks okey. I hope the change is satisfactory, please let me know if you have any further ideas for improvement. Thanks!

@Prithvi686

Copy link
Copy Markdown
Collaborator

@Prithvi686 thanks for catching the issue, seperated registry extraction from model import extraction now. I think registry can also be part of the SModel later but for now that looks okey. I hope the change is satisfactory, please let me know if you have any further ideas for improvement. Thanks!

@btakim thanks for the fix, it looks good to me now!

@danielratiu danielratiu merged commit 71c11c1 into mbeddr:main Jun 17, 2026
4 of 5 checks passed
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.

3 participants