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

json_deserialization: _construct_model_reference is called with generic model.Referable type #367

Closed
s-heppner opened this issue Jan 15, 2025 · 1 comment
Labels
duplicate This issue or pull request already exists enhancement Enhancement of an existing feature sdk Something to do with the `sdk` package

Comments

@s-heppner
Copy link
Contributor

s-heppner commented Jan 15, 2025

Currently, _construct_model_reference is very often called with the generic model.Referable as type_, instead of one of the more concret subclasses (e.g. model.Property).
This means, a model.ModelReference then has just the generic model.Referable as type attribute, leading to problems when trying to analyze the Reference further in the code.

I suggest to improve the calls of this deserialization function so that the actual type of the model object that the ModelReference points to is written to the ModelReference.type.

This issue was first noted in #337, where a hotfix is implemented, by inferering the type the ModelReference points to by using the last_key_type of the last Key in the References Keys.
Once this issue is solved, we should remove the then unnecessary hotfix from the _construct_model_reference method, that is marked with # TODO.

@s-heppner s-heppner added enhancement Enhancement of an existing feature sdk Something to do with the `sdk` package labels Jan 15, 2025
s-heppner pushed a commit that referenced this issue Jan 15, 2025
…ce` (#337)

Previously, if the `semantic_id` was a 
`ModelReference`, it very often had the generic 
`type` "`model.Referable`" instead of the actual
type of the object it points to. This lead to a 
bug in the AASX writer, where 
`ConceptDescription`s were not written to the 
AASX, even though they existed in the 
`ObjectStore`.

This fixes this problem by tackling three separate
points: 

1. In `json_deserialization.py`, we now infer the
`type` of the `ModelReference` via the Reference's
last `Key`. This is more of a hotfix and issue 
#367 tracks the creation of a better solution. 

2. In `aasx.py`, an unneccessary `logger.info` 
call is removed, that previously cluttered the 
output of the writer if a `semantic_id` was not 
pointing to a `ConceptDescription`, which is not
something worth noting. Furthermore, we improve
other log messages. 

3. In `examples.data._helper`, we improve the test
for `semantic_id` equality, namely by comparing
the `ModelReference`'s attributes explicitly, when
it happens to be one, instead of relying on simple
string comparision.
@s-heppner s-heppner added the duplicate This issue or pull request already exists label Jan 23, 2025
@s-heppner
Copy link
Contributor Author

This is a duplicate of #169

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
duplicate This issue or pull request already exists enhancement Enhancement of an existing feature sdk Something to do with the `sdk` package
Projects
None yet
Development

No branches or pull requests

1 participant