Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

examples: Don't assume array parameters are implemented as lists #924

Merged
merged 4 commits into from
Sep 24, 2024
Merged

examples: Don't assume array parameters are implemented as lists #924

merged 4 commits into from
Sep 24, 2024

Conversation

bkeryan
Copy link
Collaborator

@bkeryan bkeryan commented Sep 23, 2024 •
edited
Loading

What does this Pull Request accomplish?

niscope_acquire_waveform:

  • Explicitly convert measurement_pins to a list before concatenating it to another list.
  • Update the type hint to Sequence[str]. Iterable[str] is also okay.

service/tests:

  • Change configuration parameter type hints from List[T] to Iterable[T]. Sequence[T] is also okay.
  • Fix incorrect type hints and weird conversions in nidcpower_measurement
  • Fix incorrect type hints in nidigital_measurement
  • Fix some test services that didn't have type hints and were returning a float instead of a tuple

Why should this Pull Request be merged?

After #767, array parameters are represented as a protobuf-specific Sequence[T] such as RepeatedScalarFieldContainer[T].

What testing has been done?

Ran updated examples.

@github-actions GitHub Actions
Copy link

github-actions bot commented Sep 23, 2024 •
edited
Loading

Test Results

    40 files  ± 0      40 suites  ±0   58m 20s ⏱️ + 6m 45s
   693 tests + 1     693 ✅ + 1      0 💤 ±0  0 ❌ ±0 
16 830 runs  +10  15 760 ✅ +10  1 070 💤 ±0  0 ❌ ±0 

Results for commit e995379. ± Comparison against base commit 868b6db.

♻️ This comment has been updated with latest results.

@bkeryan bkeryan merged commit d441367 into main Sep 24, 2024
17 checks passed
@bkeryan bkeryan deleted the users/bkeryan/dont-assume-list-parameters branch September 24, 2024 22:12
bkeryan added a commit that referenced this pull request Sep 24, 2024
@bkeryan
examples: Don't assume array parameters are implemented as lists (#924)
be5f500 
* examples: Update niscope_acquire_waveform to expect Sequence[str], not List[str]

* examples: Update sample_measurement to use Sequence[T] instead of Iterable[T]

* service/tests: Fix type hints

* examples: Undo Iterable->Sequence change in sample_measurement

(cherry picked from commit d441367)
@bkeryan bkeryan mentioned this pull request Sep 24, 2024
Merged
1 task
bkeryan added a commit that referenced this pull request Sep 26, 2024
@bkeryan
examples: Don't assume array parameters are implemented as lists (#924)
d2b6e39 
* examples: Update niscope_acquire_waveform to expect Sequence[str], not List[str]

* examples: Update sample_measurement to use Sequence[T] instead of Iterable[T]

* service/tests: Fix type hints

* examples: Undo Iterable->Sequence change in sample_measurement

(cherry picked from commit d441367)
bkeryan added a commit that referenced this pull request Sep 26, 2024
@bkeryan
[releases/2.1] Cherry-pick: examples: Don't assume array parameters a…
c2a1f00 
…re implemented as lists (#936)

examples: Don't assume array parameters are implemented as lists (#924)

* examples: Update niscope_acquire_waveform to expect Sequence[str], not List[str]

* examples: Update sample_measurement to use Sequence[T] instead of Iterable[T]

* service/tests: Fix type hints

* examples: Undo Iterable->Sequence change in sample_measurement

(cherry picked from commit d441367)
@bkeryan bkeryan linked an issue Sep 27, 2024 that may be closed by this pull request
niscope_acquire_waveform example fails with "unsupported operand type(s)" error #923
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dixonjoel dixonjoel dixonjoel approved these changes

@csjall csjall Awaiting requested review from csjall csjall is a code owner

@mshafer-NI mshafer-NI Awaiting requested review from mshafer-NI mshafer-NI is a code owner

@nstokes-nati nstokes-nati Awaiting requested review from nstokes-nati nstokes-nati is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

niscope_acquire_waveform example fails with "unsupported operand type(s)" error
2 participants
@bkeryan @dixonjoel