Skip to content

model.base: add id_short path resolution#249

Merged
s-heppner merged 5 commits intoeclipse-basyx:mainfrom
rwth-iat:feature/id_short_path_resolution
Mar 14, 2024
Merged

model.base: add id_short path resolution#249
s-heppner merged 5 commits intoeclipse-basyx:mainfrom
rwth-iat:feature/id_short_path_resolution

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.
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
Member

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
Member

Other than that, this looks good to me!

@s-heppner s-heppner merged commit 04e06d6 into eclipse-basyx:main Mar 14, 2024
@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