Skip to content

Add Support for Batch Publishing Non-Scalar Measurement Values#133

Open
hunter-ni wants to merge 23 commits into
mainfrom
users/hunter-ni/publish-measurement-batch-non-scalar
Open

Add Support for Batch Publishing Non-Scalar Measurement Values#133
hunter-ni wants to merge 23 commits into
mainfrom
users/hunter-ni/publish-measurement-batch-non-scalar

Conversation

@hunter-ni
Copy link
Copy Markdown
Contributor

@hunter-ni hunter-ni commented May 19, 2026

What does this Pull Request accomplish?

This set of changes updates the MDS Python API to support the batch publishing of non-scalar measurement values (e.g., AnalogWaveform). This is in addition to the pre-existing support for batch publishing scalar measurement values.

This reflects the addition of this support to the service API itself in ni/ni-apis#161.

Implementation

  • Updates _grpc_conversion.py to handle an Iterable of the various supported non-scalar data types being supplied as the values object being passed into publish_measurement_batch.
    • This use of Iterable is consistent with our existing support of an Iterable of float, int, str, and bool being supplied for batch publishing scalar values.

Why should this Pull Request be merged?

This PR should be submitted to maintain feature support/consistency between the MDS Python API and the DataStoreService itself.

What testing has been done?

  • Added new unit tests related to populating the PublishMeasurementBatchRequest appropriately from batched, non-scalar values supplied to publish_measurement_batch
    • Includes error cases for heterogeneous Iterables and unsupported types being supplied
  • Added acceptance tests verifying that non-scalar measurement data can successfully be batch published and then read back from MDS
  • Added a notebook example describing and exercising both the scalar and non-scalar data cases of publish_measurement_batch (in addition to publish_condition_batch).

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This pull request extends the NI DataStore Python client’s batch publishing support to include non-scalar measurement value types (e.g., AnalogWaveform, Vector, XYData) in addition to the existing scalar batch support, aligning the client with updated service API capabilities.

Changes:

  • Updated gRPC request population logic to accept Iterables of supported non-scalar measurement types and map them into the appropriate PublishMeasurementBatchRequest fields.
  • Added extensive unit and acceptance tests validating successful conversions/publishing and error handling for heterogeneous/unsupported iterables.
  • Added a notebook example demonstrating batch publishing for both scalar and non-scalar measurements (and conditions).

Reviewed changes

Copilot reviewed 6 out of 7 changed files in this pull request and generated no comments.

Show a summary per file
File Description
tests/unit/data/test_publish_measurement.py Updates an assertion to match the new unsupported-iterable error message format.
tests/unit/data/test_grpc_conversion.py Adds unit tests covering batch conversion for multiple non-scalar types and error cases.
tests/acceptance/test_publish_measurement_batch_and_read_data.py Adds acceptance tests for batch publishing/read-back of non-scalar values (e.g., AnalogWaveform, per-iteration Vector).
src/ni/datastore/data/_grpc_conversion.py Implements iterable-of-non-scalar handling for publish_measurement_batch request construction.
pyproject.toml Bumps ni-measurements-data-v1-client minimum prerelease version to pick up new API support.
poetry.lock Updates locked dependency versions consistent with the updated client/proto requirements.
examples/notebooks/publish/publish_batch.ipynb New notebook documenting and exercising scalar + non-scalar batch publishing flows.

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

@hunter-ni hunter-ni marked this pull request as ready for review May 20, 2026 17:43
@hunter-ni hunter-ni requested review from a team and mjohanse-emr May 20, 2026 17:43
Comment thread examples/notebooks/publish/publish_batch.ipynb
Comment thread src/ni/datastore/data/_grpc_conversion.py Outdated
assert vector == expected_vector


def test___publish_batch_double_analog_waveforms___read_measurement_value_returns_each_analog_waveform(
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

do we want any tests for spectrum or XY data?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

My initial inclination was to have more exhaustive coverage at the level of test_grpc_conversion.py and then just have representative acceptance-level tests for a couple of selected non-scalar types. We can see if others feel strongly on this one way or the other.

Comment thread tests/unit/data/test_grpc_conversion.py
values_iterator = iter(values)
try:
vector = Vector(values)
first_value = next(values_iterator)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This is a really long if / elif block. At a minimum, maybe we could split up the contents of the if blocks into small helper methods. Better is probably some kind of a strategy to produce the data to publish. I'll be out after today, so feel free to discuss with others or override this, but as is, it's pretty unreadable.

Copy link
Copy Markdown
Collaborator

@dixonjoel dixonjoel left a comment

Choose a reason for hiding this comment

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

Approved with a suggestion for code cleanup. I'm not doing 'Request Changes' since I'll be out on vacation as of tomorrow, so I'll trust you to address those.

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.

5 participants