Skip to content

Conversation

@mjohanse-emr
Copy link
Contributor

@mjohanse-emr mjohanse-emr commented Jun 16, 2025

What does this Pull Request accomplish?

Creates python<-->gRPC converters for waveform.AnalogWaveform and bintime.DateTime.

Two converters created:
AnalogWaveform (python) <--> DoubleAnalogWaveform (protobuf)

  • Only handles AnalogWaveforms with type np.float64
  • Only sets timing parameters if available (t0, dt)
  • Converts ExtendedPropertyDictionary to the associated protobuf field attributes

DateTime (python) <--> PrecisionTimestamp (protobuf)

  • Converts between ticks and (whole seconds, fractional seconds)
  • Will eventually use to_tuple and from_tuple() entrypoints from nitypes

Why should this Pull Request be merged?

Contributes to development for AB#3161134

What testing has been done?

I added unit tests for the AnalogWaveform converter (both directions). All unit tests pass.

@mjohanse-emr mjohanse-emr changed the title Users/mjohanse/more pb converters Add python<-->gRPC converters for AnalogWaveform and DateTime. Jun 16, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Jun 16, 2025

Test Results

 10 files  ±  0   10 suites  ±0   15s ⏱️ -6s
 96 tests + 16   96 ✅ + 16  0 💤 ±0  0 ❌ ±0 
930 runs  +160  930 ✅ +160  0 💤 ±0  0 ❌ ±0 

Results for commit bda3e42. ± Comparison against base commit 9afcc8a.

This pull request removes 2 and adds 18 tests. Note that renamed tests count towards both.
tests.unit.test_streamlit_panel ‑ test___get_value_returns_default_when_value_not_set
tests.unit.test_streamlit_panel ‑ test___set_value___get_value_ignores_default
tests.unit.test_protobuf_type_conversion ‑ test___analog_waveform_non_zero_samples___convert___valid_protobuf
tests.unit.test_protobuf_type_conversion ‑ test___analog_waveform_samples_only___convert___valid_protobuf
tests.unit.test_protobuf_type_conversion ‑ test___analog_waveform_with_extended_properties___convert___valid_protobuf
tests.unit.test_protobuf_type_conversion ‑ test___analog_waveform_with_standard_timing___convert___valid_protobuf
tests.unit.test_protobuf_type_conversion ‑ test___dbl_analog_wfm_with_attributes___convert___valid_python_object
tests.unit.test_protobuf_type_conversion ‑ test___dbl_analog_wfm_with_timing___convert___valid_python_object
tests.unit.test_protobuf_type_conversion ‑ test___dbl_analog_wfm_with_timing_no_dt___convert___valid_python_object
tests.unit.test_protobuf_type_conversion ‑ test___dbl_analog_wfm_with_timing_no_t0___convert___valid_python_object
tests.unit.test_protobuf_type_conversion ‑ test___dbl_analog_wfm_with_y_data___convert___valid_python_object
tests.unit.test_protobuf_type_conversion ‑ test___default_analog_waveform___convert___valid_protobuf
…

♻️ This comment has been updated with latest results.

Copy link
Contributor Author

@mjohanse-emr mjohanse-emr left a comment

Choose a reason for hiding this comment

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

Just some explanatory comments to inform reviewers.

Michael Johansen added 2 commits June 16, 2025 16:08
Signed-off-by: Michael Johansen <[email protected]>
Signed-off-by: Michael Johansen <[email protected]>
Copy link
Contributor Author

@mjohanse-emr mjohanse-emr left a comment

Choose a reason for hiding this comment

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

Implementation details I'm not sure about that I am looking to get feedback on.

@mjohanse-emr mjohanse-emr marked this pull request as ready for review June 16, 2025 21:20
@bkeryan
Copy link
Collaborator

bkeryan commented Jun 17, 2025

This PR was in draft mode when I checked for PRs to review earlier, so I will review it tomorrow.

@mjohanse-emr mjohanse-emr requested a review from bkeryan June 17, 2025 15:51
Michael Johansen added 2 commits June 17, 2025 15:40
@mjohanse-emr mjohanse-emr requested a review from bkeryan June 17, 2025 20:49
@mjohanse-emr mjohanse-emr merged commit e2793bc into main Jun 18, 2025
14 checks passed
@mjohanse-emr mjohanse-emr deleted the users/mjohanse/more_pb_converters branch June 18, 2025 21:43
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.

4 participants