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

model.base: add id_short path resolution #249

Merged

Conversation

jkhsjdhjs
Copy link
Contributor

Resolution of id_short paths is added via UniqueIdShortNamespace.get_referable(), such that it can be used on every object, that spans such a namespace. ModelReference.resolve() is simplified to make use of this new functionality. Furthermore, tests for this are added.

Fix #214

Resolution of id_short paths is added via
`UniqueIdShortNamespace.get_referable()`, such that it can be used on
every object, that spans such a namespace. `ModelReference.resolve()`
is simplified to make use of this new functionality. Furthermore,
tests for this are added.
Show the object, where the resolution failed, in the error messages.
basyx/aas/model/base.py Outdated Show resolved Hide resolved
Comment on lines +1054 to +1055
item = UniqueIdShortNamespace.get_referable(item, # type: ignore[arg-type]
map(lambda k: k.value, self.key[1:]))
Copy link
Contributor

Choose a reason for hiding this comment

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

This is very elegant but indeed really hard to understand.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think the map(lambda... expression is pretty clean, but calling get_referable() and manually supplying self as item, because item is not necessarily a UniqueIdShortNamespace, is not. However, I think it's acceptable because get_referable() checks the type of self in the first iteration and because it's all documented via comments, in ModelReference.resolve() as well as UniqueIdShortNamespace.get_referable(). It's the only solution I could come up with that doesn't require duplicate error messages, i.e. having the same check with the same error message in both functions.

@s-heppner
Copy link
Contributor

Other than that, this looks good to me!

@s-heppner s-heppner merged commit 04e06d6 into eclipse-basyx:main Mar 14, 2024
8 checks passed
@s-heppner s-heppner deleted the feature/id_short_path_resolution branch March 14, 2024 13:24
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.

Resolve id_short path with one function call
2 participants