You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
APIs allow repeated capabilities to access attributes and methods that don't support them #2038
As of nimi-python 1.4.7, the way repeated capabilities work is thus:
When a _RepeatedCapabilities object, such as Session.channels, is accessed, it returns a _SessionBase object with _SessionBase.repeated_capability updated, based on what's passed to index operator [].
When setting an attribute or calling a method using a repeated capability, _SessionBase.repeated_capability is passed to the method or SetAttribute method.
_SessionBase is one big collection of properties and methods that support repeated capabilities, with no distinction of which repeated capabilities are supported. As an example, the nifgen API will allow a user to set an attribute that supports the data_marker repeated capability with the script_trigger repeated capability. Because we allow this, the user may be puzzled when they get back an error.
When we first developed nimi-python, we didn't have metadata stating what driver functions/attributes supported which repeated capabilities.
But now we do.
A better design would be, in my opinion, to generate a Channel, Pin, MarkerEvent, etc. class with only those functions and attributes that apply. The repeated capabilities "container" would return one of these, rather than a _SessionBase.
FWIW, I think the title of this issue makes the problem seem bigger than it really is.
It's true that the Python bindings will allow the client to call into the driver with an invalid set of arguments, but the driver runtime will still catch it and report a helpful error. It's just that we could improve on it and make it impossible to call the driver runtime in such way.
Changing the repeated capability object types would have more value if you added type hints. :) Then it would affect which properties show up in autocomplete.
FWIW, I think the title of this issue makes the problem seem bigger than it really is.
Perhaps, but I' m not sure how else to word it. I did label it as an enhancement, rather than a bug.
It's true that the Python bindings will allow the client to call into the driver with an invalid set of arguments, but the driver runtime will still catch it and report a helpful error.
Yes, the driver should still error.
I don't have an example handy comment on how helpful the error would be.
Changing the repeated capability object types would have more value if you added type hints. :) Then it would affect which properties show up in autocomplete.
Yes, I think there's at least some overlap with #2019.
Activity
marcoskirsch commentedon Feb 9, 2024
When we first developed nimi-python, we didn't have metadata stating what driver functions/attributes supported which repeated capabilities.
But now we do.
A better design would be, in my opinion, to generate a Channel, Pin, MarkerEvent, etc. class with only those functions and attributes that apply. The repeated capabilities "container" would return one of these, rather than a _SessionBase.
This is not a trivial change to make though.
ni-jfitzger commentedon Feb 9, 2024
Agreed, this would be a major change.
marcoskirsch commentedon Feb 9, 2024
FWIW, I think the title of this issue makes the problem seem bigger than it really is.
It's true that the Python bindings will allow the client to call into the driver with an invalid set of arguments, but the driver runtime will still catch it and report a helpful error. It's just that we could improve on it and make it impossible to call the driver runtime in such way.
bkeryan commentedon Feb 9, 2024
Changing the repeated capability object types would have more value if you added type hints. :) Then it would affect which properties show up in autocomplete.
ni-jfitzger commentedon Feb 9, 2024
Perhaps, but I' m not sure how else to word it. I did label it as an enhancement, rather than a bug.
Yes, the driver should still error.
I don't have an example handy comment on how helpful the error would be.
ni-jfitzger commentedon Feb 9, 2024
Yes, I think there's at least some overlap with #2019.