Skip to content

Fix name generation for parameter in ChannelTuple

Eugene Huang requested to merge eugenhu/quantify-core:generate-name-fix into main

Explanation of changes

My proposed fix for #373 (closed).

Motivation of changes

ChannelTuple also has no name attribute, so I've used short_name in _generate_long_name() and _generate_name(). I'm not sure what the rationale with (on line 1408):

# Remove the parent name in the case where the parent name is also present in
# the child names
for i, _ in enumerate(subnames):
    for j in range(i):
        subnames[i] = subnames[i].replace(subnames[j] + "_", "")

was but I've removed this part since presumably short_name won't contain the parent name? In any case, the old code can accidentally remove part of a settable name just because it matches a submodule/parameter name. E.g. a parameter named 'sensor_error_value', will be renamed in the dataset to just 'sensor_value' if the root instrument or a parent module name is called 'error' for whatever reason.

Also, the logic of the old code only checked the immediate parameters of the channels in a ChannelTuple. I've made it also recurse through each channel, like the current logic for submodules.

I have not run the test suite.

Merge checklist

See also merge request guidelines

  • Merge request has been reviewed (in-depth by a knowledgeable contributor), and is approved by a project maintainer.
  • New code is covered by unit tests (or N/A).
  • New code is documented and docstrings use numpydoc format (or N/A).
  • New functionality: considered making private instead of extending public API (or N/A).
  • Public API changed: added @deprecated (or N/A).
  • Newly added/adjusted documentation and docstrings render properly (or N/A).
  • Pipeline fix or dependency update: post in #software-for-developers channel to merge main back in or update local packages (or N/A).
  • Tested on hardware (or N/A).
  • CHANGELOG.md and AUTHORS.md have been updated (or N/A).
  • Windows tests in CI pipeline pass (manually triggered by maintainers before merging).
    • Maintainers do not hit Auto-merge, we need to actively check as manual tests do not block pipeline

For reference, the issues workflow is described in the contribution guidelines.

Merge request reports