Skip to content

Custom object expansion for repeated capabilities #2100

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

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

Vaishnavigupta1312
Copy link
Contributor

@Vaishnavigupta1312 Vaishnavigupta1312 commented May 16, 2025

  • This contribution adheres to CONTRIBUTING.md.

  • I've updated CHANGELOG.md if applicable.

  • I've added tests applicable for this pull request

What does this Pull Request accomplish?

Currently to set an attribute with repeated capability, we create a new _SessionBase instance with repeated_capability information passed at the time of initalization.
The information of repeated capability stored in this new _SessionBase instance is used when dispatching call to the interpreter session to set the value in the driver.
Repeated Capabilities can be handled in a way that avoids going down this route.

  • Adding new class _RepeatedCapabilityBase to serve as the base class for each of the repeated capability type, for eg _RepeatedCapablityScript for script.
  • These new capability specific classes will contain the attributes supporting that repeated capability.
  • Example of existing flow using a script attribute.
    image
  • Proposed flow using same script example, changes are highlighted using red font.
    image

Changes are made to the helper scripts and session mako template to support custom object expansion of the applicable only attributes.

List issues fixed by this Pull Request below, if any.

What testing has been done?

  • Successful tox build without any errors.
  • No failure of newly added unit test.

@codecov-commenter
Copy link

codecov-commenter commented May 16, 2025

Codecov Report

Attention: Patch coverage is 60.00000% with 4 lines in your changes missing coverage. Please review.

Project coverage is 91.33%. Comparing base (9efe042) to head (2faedc0).
Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
build/helper/documentation_helper.py 25.00% 3 Missing ⚠️
build/helper/metadata_filters.py 50.00% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2100      +/-   ##
==========================================
- Coverage   91.34%   91.33%   -0.01%     
==========================================
  Files          66       66              
  Lines       16292    16301       +9     
==========================================
+ Hits        14882    14889       +7     
- Misses       1410     1412       +2     
Flag Coverage Δ
codegenunittests 84.85% <60.00%> (-0.11%) ⬇️
nidcpowersystemtests 94.59% <ø> (+0.04%) ⬆️
nidcpowerunittests 89.53% <ø> (ø)
nidigitalsystemtests 92.26% <ø> (ø)
nidigitalunittests 68.44% <ø> (ø)
nidmmsystemtests 92.72% <ø> (ø)
nifakeunittests 87.24% <ø> (ø)
nifgensystemtests 94.86% <ø> (ø)
nimodinstsystemtests 73.85% <ø> (ø)
nimodinstunittests 94.20% <ø> (ø)
niscopesystemtests 92.94% <ø> (ø)
niscopeunittests 43.20% <ø> (ø)
nisesystemtests 91.50% <ø> (ø)
niswitchsystemtests 82.03% <ø> (ø)
nitclksystemtests 94.87% <ø> (ø)
nitclkunittests 98.26% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
build/helper/__init__.py 100.00% <100.00%> (ø)
build/helper/documentation_snippets.py 85.18% <100.00%> (+0.27%) ⬆️
build/helper/metadata_add_all.py 80.84% <100.00%> (+0.08%) ⬆️
build/helper/metadata_filters.py 78.72% <50.00%> (-0.63%) ⬇️
build/helper/documentation_helper.py 88.12% <25.00%> (-0.32%) ⬇️

... and 1 file with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9efe042...2faedc0. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Comment on lines 315 to 319
complete_rep_cap_list = [
current_rep_cap + self._separator + rep_cap
for current_rep_cap in self._current_repeated_capability_list
for rep_cap in rep_caps_list
]
Copy link
Collaborator

Choose a reason for hiding this comment

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

This list comprehension exists to support the chaining of repeated capabilities, i.e. session.sites[0, 1].pins['PinA', 'PinB'].ppmu_source().
See #1291

This new implementation won't work with chaining.
You can't call .pins in a class where we don't define properties/attributes for each of the other rep cap classes.

Comment on lines 320 to 321
object.__setattr__(self, '_current_repeated_capability_list', complete_rep_cap_list)
self._current_repeated_capability_list = complete_rep_cap_list
Copy link
Collaborator

@ni-jfitzger ni-jfitzger May 16, 2025

Choose a reason for hiding this comment

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

Why is this being set twice?

Comment on lines 453 to 461
def filter_rep_cap_supported_attributes(attributes, rep_cap_name):
'''Returns attribute metadata only for those attributes that support the specified repeated capability.
Args:
attributes: Dictionary of attribute metadata.
rep_cap_name: The name of the repeated capability to filter by.
Returns:
Dictionary of attributes that support the specified repeated capability.
'''
return {k: v for k, v in attributes.items() if rep_cap_name in v.get('supported_rep_caps', [])}
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is good, but what about filter_rep_cap_supported_functions?

Comment on lines 80 to 89

Example: :py:attr:`my_session.waveform[ ... ].waveform_runtime_scaling`

To set/get on all waveform, you can call the property directly on the :py:class:`nirfsg.Session`.

Example: :py:attr:`my_session.waveform_runtime_scaling`
'''
waveform_signal_bandwidth = _attributes.AttributeViReal64(1150264)
'''Type: float

Specifies the bandwidth of the arbitrary signal. This value must be less than or equal to (0.8× iq_rate).

**Units**: hertz (Hz)

**Supported Devices:** PXIe-5820/5830/5831/5832/5840/5841/5842/5860

Tip:
This property can be set/get on specific waveform within your :py:class:`nirfsg.Session` instance.
Use Python index notation on the repeated capabilities container waveform to specify a subset.

Example: :py:attr:`my_session.waveform[ ... ].waveform_signal_bandwidth`

To set/get on all waveform, you can call the property directly on the :py:class:`nirfsg.Session`.

Example: :py:attr:`my_session.waveform_signal_bandwidth`
'''
waveform_waveform_size = _attributes.AttributeViInt32(1150297)
'''Type: int

Specifies the size of the waveform specified by an active channel.

**Supported Devices:** PXIe-5820/5830/5831/5832/5840/5841/5841 with PXIe-5655/5842/5860

Tip:
This property can be set/get on specific waveform within your :py:class:`nirfsg.Session` instance.
Use Python index notation on the repeated capabilities container waveform to specify a subset.

Example: :py:attr:`my_session.waveform[ ... ].waveform_waveform_size`

To set/get on all waveform, you can call the property directly on the :py:class:`nirfsg.Session`.

Example: :py:attr:`my_session.waveform_waveform_size`
'''
def __init__(self, session, repeated_capability_list):
object.__setattr__(self, '_session', session)
object.__setattr__(self, '_repeated_capability_list', repeated_capability_list)
object.__setattr__(self, '_prefix', 'waveform::')
object.__setattr__(self, '_current_repeated_capability_list', repeated_capability_list if len(repeated_capability_list) > 0 else [''])
object.__setattr__(self, '_separator', '')

def __setattr__(self, key, value):
if key not in dir(self):
raise AttributeError("'{0}' object has no attribute '{1}'".format(type(self).__name__, key))
object.__setattr__(self, key, value)

def __getitem__(self, repeated_capability):
'''Set/get properties or call methods with a repeated capability (i.e. channels)'''
rep_caps_list = _converters.convert_repeated_capabilities(repeated_capability, self._prefix)
complete_rep_cap_list = [current_rep_cap + self._separator + rep_cap for current_rep_cap in self._current_repeated_capability_list for rep_cap in rep_caps_list]
complete_rep_cap_list = [
current_rep_cap + self._separator + rep_cap
for current_rep_cap in self._current_repeated_capability_list
for rep_cap in rep_caps_list
]
object.__setattr__(self, '_current_repeated_capability_list', complete_rep_cap_list)
self._current_repeated_capability_list = complete_rep_cap_list

return _SessionBase(
repeated_capability_list=complete_rep_cap_list,
all_channels_in_session=self._session._all_channels_in_session,
interpreter=self._session._interpreter,
freeze_it=True
)
return self

def _get_attribute_vi_real64(self, attribute):
repeated_capability = ','.join(self._current_repeated_capability_list)
value = self._session._interpreter.get_attribute_vi_real64(repeated_capability, attribute)
return value

# This is a very simple context manager we can use when we need to set/get attributes
# or call functions from _SessionBase that require no channels. It is tied to the specific
# implementation of _SessionBase and how repeated capabilities are handled.
class _NoChannel(object):
def __init__(self, session):
self._session = session
def _set_attribute_vi_real64(self, attribute, value):
repeated_capability = ','.join(self._current_repeated_capability_list)
self._session._interpreter.set_attribute_vi_real64(repeated_capability, attribute, value)

def __enter__(self):
self._repeated_capability_cache = self._session._repeated_capability
self._session._repeated_capability = ''
def _get_attribute_vi_int32(self, attribute):
repeated_capability = ','.join(self._current_repeated_capability_list)
value = self._session._interpreter.get_attribute_vi_int32(repeated_capability, attribute)
return value

def __exit__(self, exc_type, exc_value, traceback):
self._session._repeated_capability = self._repeated_capability_cache
Copy link
Collaborator

Choose a reason for hiding this comment

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

This class is for calling a function or attribute that does not support repeated capabilities, while in the middle of a fancy function that may have had repeated capabilties passed in.

Even if nirfsg doesn't happen to need it at the moment, I'm not sure it's okay to just blindly omit it. We need to reason through how such a use case would work without it. If nothing else, fancy function code would need to change to not use this, but I'm not sure it would work.

@ni-jfitzger
Copy link
Collaborator

The description for this PR is not sufficient.
I'd like to see a theory of operation. I don't think you've reasoned through all of the use cases.
You could probably stand to add some more tests. If nothing else, you should test your new error behavior.

Comment on lines 788 to 813
def _get_attribute_vi_real64(self, attribute):
repeated_capability = ','.join(self._current_repeated_capability_list)
value = self._session._interpreter.get_attribute_vi_real64(repeated_capability, attribute)
return value

def _set_attribute_vi_real64(self, attribute, value):
repeated_capability = ','.join(self._current_repeated_capability_list)
self._session._interpreter.set_attribute_vi_real64(repeated_capability, attribute, value)

def _get_attribute_vi_int32(self, attribute):
repeated_capability = ','.join(self._current_repeated_capability_list)
value = self._session._interpreter.get_attribute_vi_int32(repeated_capability, attribute)
return value

def _set_attribute_vi_int32(self, attribute, value):
repeated_capability = ','.join(self._current_repeated_capability_list)
self._session._interpreter.set_attribute_vi_int32(repeated_capability, attribute, value)

def _get_attribute_vi_string(self, attribute):
repeated_capability = ','.join(self._current_repeated_capability_list)
value = self._session._interpreter.get_attribute_vi_string(repeated_capability, attribute)
return value

def _set_attribute_vi_string(self, attribute, value):
repeated_capability = ','.join(self._current_repeated_capability_list)
self._session._interpreter.set_attribute_vi_string(repeated_capability, attribute, value)
Copy link
Collaborator

Choose a reason for hiding this comment

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

If you're going to put these in every rep cap class, why not have a base class to inherit them from?

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.

APIs allow repeated capabilities to access attributes and methods that don't support them
4 participants