Reverse Indices for Digital Waveform Signals

[mikeprosserni]

• This contribution adheres to CONTRIBUTING.md.

What does this Pull Request accomplish?

In NI Digital Waveforms, the signals are typically ordered with the least significant bit last, and that least significant line is line0. That means, for example, if you read digital data from a 32 bit port, the line numbers (aka signal numbers) count down from 31 to 0. Here's what that looks like in LabVIEW. (Note, this is simulated hardware where the samples count up from zero)

In our .NET Digital Waveforms, we match this 'reversed' signal ordering using logic like this (from line 152 in DigitalData.cs)

        //------------------------------------------------------------------------------------------
        // breeve [ 2/3/2005 8:34:00 AM ] - The indexer uses subtraction to
        // give the user the illusion that the least significant signal is at
        // index [0]. Hardware does not return the least significant signal
        // at index[0], thus we need to do the math here. It has been shown that doing
        // subtraction in the indexer is faster in common cases than manually going through
        // the data at interop time and rearranging the signals.
        //------------------------------------------------------------------------------------------
        public DigitalState this[int sample, int signal]
        {
            get
            {
                return _data[sample, _signalIndexCount - signal];
            }

            set
            {
                this.GuardWritable();
                Debug.Assert(Enum.IsDefined(typeof(DigitalState), value));

                _data[sample, _signalIndexCount - signal] = value;
            }
        }

We should do the same thing in our Python Digital Waveforms.

To accomplish this, I've added a signal.column_index property alongside the signal.signal_index property. The signal_index represents the line number, and counts down, while the column_index represents the position in the underlying signals.data array, and counts up. These two indices are reversed with respect to each other:

def _reverse_index(self, index: int) -> int:
    """Convert a signal index to a data index, or vice versa."""
    assert 0 <= index < self.signal_count
    return self.signal_count - 1 - index

I have updated _signal.py, _signal_collection.py, and _waveform.py to set and use the correct index when accessing things like signal.data and signal.name. (Note that the strings in NI_LineNames match the ordering of column_index, and are reversed with respect to signal_index)

    @property
    def data(self) -> npt.NDArray[TDigitalState]:
        """The signal data, indexed by sample."""
        return self._owner.data[:, self.column_index]

    @property
    def name(self) -> str:
        """The signal name."""
        return self._owner._get_line_name(self.column_index)

    @name.setter
    def name(self, value: str) -> None:
        self._owner._set_line_name(self.column_index, value)

I have also updated documentation to explain the difference between the two indices.

Here's a summary of all of the changes:

• Added signal.column_index
• Updated signal.data, signal.name, and signal.__reduce__() to use column_index
• Updated signal_collection.__getitem__() to set and use column_index
• Added DigitalWaveformFaiulre.column_index
• Updated documentation for the DigitalWaveform class
• Renamed waveform._signal_names to waveform._line_names
• Renamed waveform._get_signal_name() and set_signal_name() to get_line_name() and set_line_name()
• Updated waveform.test() to include the column_index in the DigitalWaveformFalure report

Why should this Pull Request be merged?

AB#3178052

What testing has been done?

Added/Updated auto tests:

test___int_index___signals_getitem___returns_signal
test___negative_int_index___signals_getitem___returns_signal
test___str_index___signals_getitem___returns_signal
test___slice_index___signals_getitem___returns_signal
test___negative_slice_index___signals_getitem___returns_signal
test___signal___set_signal_name___sets_name
test___signal_with_line_names___get_signal_name___returns_line_name
test___waveform___get_signal_data___returns_line_data
test___waveform___get_data_with_line_index___returns_line_data
test___various_values___repr___looks_ok

test___different_data___test___reports_failures
test___shifted_different_data___test___reports_shifted_failures

Signal name caching issue

While updating tests in nidaqmx-python, I noticed that the signal.name value (which is cached) wasn't being cleared/updated when waveform.extended_properties["NI_LineNames"] is updated.

I added a test test___signal_with_line_names___change_line_names_property___signal_returns_new_line_name to demonstrate the issue here in nitypes-python.

I fixed the issue by adding a _DigitalWaveformExtendedProperties class, which just clears self._waveform._line_names when ["NI_LineNames"] is changed.

[github-actions]

Test Results

    56 files  ±  0      56 suites  ±0   25m 31s ⏱️ +16s
 2 276 tests + 20   2 276 ✅ + 20  0 💤 ±0  0 ❌ ±0 
65 170 runs  +580  65 170 ✅ +580  0 💤 ±0  0 ❌ ±0 

Results for commit 7adf82c. ± Comparison against base commit 9713651.

This pull request removes 4 and adds 24 tests. Note that renamed tests count towards both.

tests.unit.waveform.test_digital_waveform ‑ test___various_values___repr___looks_ok[value13-nitypes.waveform.DigitalWaveform(3, 8, data=array([[1, 0, 0, 0, 0, 0, 0, 0], [0, 1, 0, 0, 0, 0, 0, 0], [1, 1, 0, 0, 0, 0, 0, 0]], dtype=uint8), timing=nitypes.waveform.Timing(nitypes.waveform.SampleIntervalMode.REGULAR, sample_interval=datetime.timedelta(microseconds=1000)))]
tests.unit.waveform.test_digital_waveform ‑ test___various_values___repr___looks_ok[value14-nitypes.waveform.DigitalWaveform(3, 8, data=array([[1, 0, 0, 0, 0, 0, 0, 0], [0, 1, 0, 0, 0, 0, 0, 0], [1, 1, 0, 0, 0, 0, 0, 0]], dtype=uint8), extended_properties={'NI_ChannelName': 'Dev1/ai0', 'NI_UnitDescription': 'Volts'})]
tests.unit.waveform.test_digital_waveform ‑ test___various_values___repr___looks_ok[value15-[nitypes.waveform.DigitalWaveform(3, 8, data=array([[1, 0, 0, 0, 0, 0, 0, 0], [0, 1, 0, 0, 0, 0, 0, 0], [1, 1, 0, 0, 0, 0, 0, 0]], dtype=uint8), timing=nitypes.waveform.Timing(nitypes.waveform.SampleIntervalMode.REGULAR, sample_interval=datetime.timedelta(microseconds=1000))), nitypes.waveform.DigitalWaveform(3, 8, data=array([[0, 0, 1, 0, 0, 0, 0, 0], [1, 0, 1, 0, 0, 0, 0, 0], [0, 1, 1, 0, 0, 0, 0, 0]], dtype=uint8), timing=nitypes.waveform.Timing(nitypes.waveform.SampleIntervalMode.REGULAR, sample_interval=datetime.timedelta(microseconds=1000)))]]
tests.unit.waveform.test_digital_waveform ‑ test___various_values___repr___looks_ok[value16-[nitypes.waveform.DigitalWaveform(3, 8, data=array([[1, 0, 0, 0, 0, 0, 0, 0], [0, 1, 0, 0, 0, 0, 0, 0], [1, 1, 0, 0, 0, 0, 0, 0]], dtype=uint8), extended_properties={'NI_ChannelName': 'Dev1/ai0', 'NI_UnitDescription': 'Volts'}), nitypes.waveform.DigitalWaveform(3, 8, data=array([[0, 0, 1, 0, 0, 0, 0, 0], [1, 0, 1, 0, 0, 0, 0, 0], [0, 1, 1, 0, 0, 0, 0, 0]], dtype=uint8), extended_properties={'NI_ChannelName': 'Dev1/ai0', 'NI_UnitDescription': 'Volts'})]]

tests.unit.waveform.test_digital_waveform ‑ test___array_subset___from_port_bitorder_little___creates_waveform_with_array_subset
tests.unit.waveform.test_digital_waveform ‑ test___array_subset___from_ports_bitorder_little___creates_waveform_with_array_subset
tests.unit.waveform.test_digital_waveform ‑ test___bool_dtype___from_port_bitorder_little___creates_waveform_with_bool_dtype
tests.unit.waveform.test_digital_waveform ‑ test___bool_dtype___from_ports_bitorder_little___creates_waveform_with_bool_dtype
tests.unit.waveform.test_digital_waveform ‑ test___int_list_and_mask___from_port_bitorder_little___creates_waveform_with_masked_lines
tests.unit.waveform.test_digital_waveform ‑ test___int_list_and_mask___from_ports_bitorder_little___creates_waveform_with_masked_lines
tests.unit.waveform.test_digital_waveform ‑ test___mask___from_port_bitorder_little___creates_waveform_with_masked_lines
tests.unit.waveform.test_digital_waveform ‑ test___masks___from_ports_bitorder_little___creates_waveform_with_masked_lines
tests.unit.waveform.test_digital_waveform ‑ test___uint16_ndarray___from_port_bitorder_little___creates_waveform_with_16_lines
tests.unit.waveform.test_digital_waveform ‑ test___uint16_ndarray___from_ports_bitorder_little___creates_waveform_with_16_lines
…

♻️ This comment has been updated with latest results.

[bkeryan]

Approved with suggestion (add test case for pickle of _on_key_changed)