Update waveform conversion code to account for new timing fields

[mjohanse-emr]

What does this Pull Request accomplish?

Updates the waveform conversion functions to handle the following new timing fields:

• timestamp
• time_offset
• timestamps

NI-APIs change to add the new fields

Unit tests have been added to test conversion cases specific to these parameters.
Existing unit tests have been updated where necessary (example, don't expect and exception for irregular timing anymore).

Similar change in the C# API

Why should this Pull Request be merged?

To establish parity between our python API and our C# API.
To allow users to publish Irregular Timing waveforms and "Relative Timing" waveforms.

What testing has been done?

Many new tests have been added that were inspired by the C# auto tests.
Existing waveform conversion tests are passing.

[github-actions]

Test Results

  120 files  ±  0    120 suites  ±0   3m 38s ⏱️ +16s
  301 tests + 50    299 ✅ + 50   2 💤 ±0  0 ❌ ±0 
3 010 runs  +500  2 980 ✅ +500  30 💤 ±0  0 ❌ ±0 

Results for commit a512e69. ± Comparison against base commit 0b84363.

This pull request removes 5 and adds 55 tests. Note that renamed tests count towards both.

tests.unit.test_waveform_conversion ‑ test___analog_waveform_with_irregular_timing___convert___raises_value_error
tests.unit.test_waveform_conversion ‑ test___digital_waveform_round_trip___convert___valid_protobuf
tests.unit.test_waveform_conversion ‑ test___float64_complex_waveform_with_irregular_timing___convert___raises_value_error
tests.unit.test_waveform_conversion ‑ test___int16_analog_waveform_with_irregular_timing___convert___raises_value_error
tests.unit.test_waveform_conversion ‑ test___int16_complex_waveform_with_irregular_timing___convert___raises_value_error

tests.unit.test_waveform_conversion ‑ test___analog_waveform_with_irregular_timing___convert___valid_protobuf
tests.unit.test_waveform_conversion ‑ test___analog_waveform_with_none_timing___round_trip___waveforms_match[0-0]
tests.unit.test_waveform_conversion ‑ test___analog_waveform_with_none_timing___round_trip___waveforms_match[0-10]
tests.unit.test_waveform_conversion ‑ test___analog_waveform_with_none_timing___round_trip___waveforms_match[100-0]
tests.unit.test_waveform_conversion ‑ test___analog_waveform_with_none_timing___round_trip___waveforms_match[100-10]
tests.unit.test_waveform_conversion ‑ test___analog_waveform_with_regular_timing___round_trip___waveforms_match[0-0]
tests.unit.test_waveform_conversion ‑ test___analog_waveform_with_regular_timing___round_trip___waveforms_match[0-10]
tests.unit.test_waveform_conversion ‑ test___analog_waveform_with_regular_timing___round_trip___waveforms_match[100-0]
tests.unit.test_waveform_conversion ‑ test___analog_waveform_with_regular_timing___round_trip___waveforms_match[100-10]
tests.unit.test_waveform_conversion ‑ test___analog_waveform_with_standard_timing___round_trip___waveforms_match
…

♻️ This comment has been updated with latest results.

[mjohanse-emr]

The round_trip tests in python are a little less dynamic than the ones in C#. Is this something that needs to be addressed?

[jasonmreding]

By dynamic, I assume you mean the tests are less data driven and are duplicated per waveform type rather than working from generic types? If so, I think moving the implementation in that direction would be great. However, I'm not going to mandate it. It's also a little above my current expertise with python in guiding you on the best way to accomplish that.

[bkeryan]

I agree that it would be helpful to reduce duplication. I think the most straightforward way to do that is to use @pytest.mark.parametrize to pass in different types of waveform objects or messages. If you can't initialize the input with a single expression, you can call a function or even pass a function as the parameter.

@pytest.mark.parametrize
(
    "waveform",
    [
        AnalogWaveform.from_array_1d(np.array([1.0, 2.0, 3.0])),
        ComplexWaveform.from_array_1d([1.5 + 2.5j, 3.5 + 4.5j], np.complex128),
        ComplexWaveform.from_array_1d([(1, 2), (3, 4)], ComplexInt32DType),
        DigitalWaveform.from_lines(np.array([[0, 1, 0], [1, 0, 1]], dtype=np.bool), signal_count=3),
    ]
)
def test_blah_blah(waveform: AnalogWaveform[Any] | ComplexWaveform[Any] | DigitalWaveform[Any]) -> None:

[bkeryan]

The C# tests are structured using the "test driver" pattern, though they aren't named that way. I think you can implement that in Python by defining a test class with subclasses, rather than test functions.

[mjohanse-emr]

I think I'll try and implement the test class suggestion. Right now, the file is just so huge and there's so much duplication. I'll see where classes get me.

[Copilot]

Pull request overview

Updates the NI waveform <-> protobuf conversion layer to support the newly-added timing fields (timestamp, time_offset, timestamps), and expands unit tests to cover regular/none/irregular timing scenarios and round-trip conversions for multiple waveform types.

Changes:

• Extend waveform-to-protobuf conversion to emit timestamp/time_offset for regular/none timing and timestamps for irregular timing.
• Extend protobuf-to-waveform conversion to interpret timestamps as irregular timing, and to incorporate timestamp + time_offset when building Timing.
• Add/adjust unit tests for the new timing-field behaviors and for round-trip conversions.

Reviewed changes

Copilot reviewed 2 out of 3 changed files in this pull request and generated 11 comments.

File	Description
packages/ni.protobuf.types/src/ni/protobuf/types/waveform_conversion.py	Implements new timing-field mapping logic in both directions (including irregular timing via timestamps).
packages/ni.protobuf.types/tests/unit/test_waveform_conversion.py	Adds/updates tests for new timing fields and round-trips across waveform types.
.gitignore	Ignores local VS Code settings directory.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[jasonmreding]

I think we need to fix the logic for the empty waveform conversion. I also think we should be consistent with normalizing unset and default timestamps in the conversion code. If we want to revisit this normalization strategy, I'm fine with that. I mainly just want to make sure the behavior is consistent with the conversions between Python and C#.

[bkeryan]

It would make sense to test all codepaths with non-zero fractional seconds: regular timing, irregular timing, to/from protobuf, etc.

I don't think it's necessary to test all waveform types with non-zero fractional seconds, because they share the same codepaths, but if it's easier to do so, then why not?