Skip to content

Fix TestResult and Step parameter ordering to make "name" required.#118

Merged
mjohanse-emr merged 4 commits intomainfrom
users/mjohanse/fix_required_params
Feb 25, 2026
Merged

Fix TestResult and Step parameter ordering to make "name" required.#118
mjohanse-emr merged 4 commits intomainfrom
users/mjohanse/fix_required_params

Conversation

@mjohanse-emr
Copy link
Copy Markdown
Collaborator

What does this Pull Request accomplish?

Re-order the parameters to the TestResult and Step so that name appears as a required parameter before the * in the parameter list.

Update all callers to put name first. This includes fixing tests and examples.

Why should this Pull Request be merged?

Fixes #103

What testing has been done?

I ran all the unit and acceptance tests.
I ran all the jupyter notebooks and examples

Signed-off-by: Michael Johansen <michael.johansen@emerson.com>
Signed-off-by: Michael Johansen <michael.johansen@emerson.com>
@mjohanse-emr mjohanse-emr requested a review from csjall as a code owner February 25, 2026 16:56
Copilot AI review requested due to automatic review settings February 25, 2026 16:56
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 PR addresses issue #103 by reordering parameters in the TestResult and Step classes to make the name parameter required instead of optional. The change moves name before the * in the parameter lists, forcing callers to provide it as a positional argument, which aligns the Python API with the proto file specifications where name is a required field.

Changes:

  • Moved name parameter before * in TestResult and Step class constructors to make it required
  • Updated all callers (tests, examples, notebooks, and documentation) to pass name as the first parameter
  • Updated docstrings to reflect the new parameter ordering

Reviewed changes

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

Show a summary per file
File Description
src/ni/datastore/data/_types/_test_result.py Moved name parameter before * in __init__, updated docstring, updated from_protobuf method, and adjusted initialization order
src/ni/datastore/data/_types/_step.py Moved name parameter before * in __init__, updated docstring, updated from_protobuf method, and adjusted initialization order
tests/unit/data/test_create_metadata.py Updated test cases to pass name as first parameter for both Step and TestResult
tests/acceptance/test_publish_with_metadata.py Updated test cases to pass name as first parameter for both Step and TestResult
examples/notebooks/voltage-regulator/publish_waveforms.ipynb Updated TestResult instantiation to pass name first; includes unintentional kernel metadata changes
examples/notebooks/query/publish_sample_data.ipynb Updated three TestResult instantiations to pass name first; includes unintentional kernel metadata changes
docs/reference/using-measurement-data-services.md Updated documentation examples to show name as the first parameter for both TestResult and Step

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

Comment thread examples/notebooks/query/publish_sample_data.ipynb
Comment thread examples/notebooks/voltage-regulator/publish_waveforms.ipynb
Comment thread src/ni/datastore/data/_types/_test_result.py
Signed-off-by: Michael Johansen <michael.johansen@emerson.com>
Signed-off-by: Michael Johansen <michael.johansen@emerson.com>
@mjohanse-emr mjohanse-emr merged commit 3f6284b into main Feb 25, 2026
29 checks passed
@mjohanse-emr mjohanse-emr deleted the users/mjohanse/fix_required_params branch February 25, 2026 21:31
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.

Bug | Mismatch between optional parameters in the proto definitions and the Python API

4 participants