Allow measurement.StartTime and t0 of a published waveform to differ#131
Open
dixonjoel wants to merge 4 commits into
Open
Allow measurement.StartTime and t0 of a published waveform to differ#131dixonjoel wants to merge 4 commits into
dixonjoel wants to merge 4 commits into
Conversation
added 2 commits
May 11, 2026 10:57
…or. Add a test to cover when neither t0 nor StartTime is specified it uses 'now'
dixonjoel
changed the title
Contributor
There was a problem hiding this comment.
Pull request overview
Updates the Python DataStore client’s publish behavior for waveforms to align with the desired service semantics when timestamp (measurement start time) and waveform t0 are both present, mismatched, or absent.
Changes:
- Removed client-side validation that raised when a provided
timestampdidn’t match waveformt0; the client now always honors the explicitly providedtimestamp. - Added/updated unit tests to verify (a) mismatched
timestampis used as-requested, and (b) when neithertimestampnor waveformt0is present, “now” is used. - Adjusted pytest configuration to avoid recursing into
.venvand to ignorePytestCollectionWarning.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| tests/unit/data/test_publish_measurement.py | Updates expectations for mismatched timestamp vs t0 and adds a deterministic test for the “no timestamp and no t0” case. |
| src/ni/datastore/data/_grpc_conversion.py | Removes the mismatch ValueError and makes timestamp selection prefer client timestamp, otherwise waveform t0, otherwise current time. |
| pyproject.toml | Updates pytest settings (including .venv recursion and warning filtering). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
added 2 commits
May 11, 2026 11:21
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this Pull Request accomplish?
See https://github.com/ni/datastore-service/issues/517. This makes the Python client match the desired behavior for publishing waveforms with and without a t0 and
StartTime. The issue discussion lists the desired behavior.Why should this Pull Request be merged?
Conform to the desired publish behavior for waveforms.
What testing has been done?
Adding a unit test for the case when neither t0 nor StartTime is specified. Updating unit test not to expect ValueError if the t0 and StartTime do not match.