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

Add setter-functions for object attributes that consist of NamespaceSets #65

Open
s-heppner opened this issue Apr 18, 2023 · 2 comments
Labels
enhancement Enhancement of an existing feature sdk Something to do with the `sdk` package

Comments

@s-heppner
Copy link
Contributor

When instantiating an AssetAdministrationShell-object, we hand over ConceptDictionarys in any kind of Iterable (See here)

def __init__(self,
                 ...
                 concept_dictionary: Iterable[concept.ConceptDictionary] = (),
                 ...):

However, internally, we create a NamespaceSet containing these ConceptDictionarys in order to be able to resolve them later:

self.concept_dictionary: base.NamespaceSet[concept.ConceptDictionary] = \
            base.NamespaceSet(self, concept_dictionary)

Now, if a user were to only look at the initialization parameters, and wanted to add a ConceptDictionary later, after initalization, they could theoretically assume, they could set it like this:

my_aas.concept_dictionary = [my_concept_dictionary]

This would have terrible results when iterating over ObjectStores containing these objects, since suddenly, we'd get the (not very helpful) error:

AttributeError: 'list' object has no attribute 'update_nss_from' 

This obviously points nowhere in the right direction.

I have the suspicion, that this is not the only time, where such a problem may arise. How can we restrrict the user from making such a mistake in the first place? Should we write getter and setter functions for these kinds of attributes? On the other hand, maybe this is something to be checked by the ObjectStore, when adding an item to it?

@s-heppner s-heppner added enhancement Enhancement of an existing feature question Further information is requested labels Apr 18, 2023
@jkhsjdhjs
Copy link
Contributor

A typechecker will catch the error of assigning a list where a NamespaceSet is expected. So if the user uses a typechecker (which they should), this won't be an issue. If the user on the other hand assigns a different NamespaceSet to the attribute, I think they should know how to use a NamespaceSet correctly, which is described by the docstring.

We could provide a setter function for the attributes where a NamespaceSet is expected just for usability reasons (I've done that for the OrderedNamespaceSet of SubmodelElementList for example).

@s-heppner
Copy link
Contributor Author

You're right, a type-checker would have caught that.

Nevertheless, I'm all for your suggestion of writing setter-functions on attributes where a NamespaceSet is expected. I'll adapt the issue accordingly.

@s-heppner s-heppner changed the title Confusion when adding ConceptDescription after instantiation of AssetAdministrationShell Add setter-functions for object attributes that consist of NamespaceSets Apr 18, 2023
@s-heppner s-heppner removed the question Further information is requested label Apr 18, 2023
@jkhsjdhjs jkhsjdhjs mentioned this issue Aug 24, 2023
26 tasks
@s-heppner s-heppner added the sdk Something to do with the `sdk` package label Nov 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Enhancement of an existing feature sdk Something to do with the `sdk` package
Projects
None yet
Development

No branches or pull requests

2 participants