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.

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

fix physical channel/device collection slice/string behavior #536

Merged
merged 3 commits into from
Mar 22, 2024

Conversation

zhindes
Copy link
Collaborator

@zhindes zhindes commented Mar 8, 2024

What does this Pull Request accomplish?

Fixes string and slice behavior of Physical Channel Collection to match that of Device Collection

  • slices return a list
  • strings are unflattened and return a list if more than 1 channel is provided

New behavior

  • For both Physical Channel and Device - raise KeyError if a string is provided that doesn't match the collection
  • Decided against this as we can't validate internal channel names, yet. Keep existing behavior.

Why should this Pull Request be merged?

Closes #392

What testing has been done?

Added new tests. Fixed existing tests that used the name of an invalid PhysicalChannel object to use the channel_names property of the collection instead.

poetry run ni-python-styleguide fix tests && poetry run ni-python-styleguide lint tests && poetry run pytest -rs -k collection
================================================= test session starts =================================================
platform win32 -- Python 3.9.13, pytest-8.0.2, pluggy-1.4.0
rootdir: C:\dev\nidaqmx-python
configfile: pyproject.toml
testpaths: tests
plugins: cov-4.1.0, mock-3.12.0
collected 2163 items / 1721 deselected / 442 selected

tests\component\system\_collections\test_device_collection.py ....................                               [  4%]
tests\component\system\_collections\test_persisted_channel_collection.py ....................                    [  9%]
tests\component\system\_collections\test_persisted_scale_collection.py ....................                      [ 13%]
tests\component\system\_collections\test_persisted_task_collection.py ....................                       [ 18%]
tests\component\system\_collections\test_physical_channel_collection.py ........................................ [ 27%]
................................................................................................................ [ 52%]
................................................................................................................ [ 77%]
........................................................................................                         [ 97%]
tests\legacy\test_system_collections.py ..........                                                               [100%]

======================================== 442 passed, 1721 deselected in 11.99s ========================================

@zhindes zhindes marked this pull request as ready for review March 8, 2024 19:34
Copy link

github-actions bot commented Mar 10, 2024

Test Results

    34 files  ±    0      34 suites  ±0   43m 15s ⏱️ + 1m 8s
 2 228 tests +  160   1 893 ✅ +  192    335 💤  -  32  0 ❌ ±0 
39 392 runs  +2 720  33 866 ✅ +3 264  5 526 💤  - 544  0 ❌ ±0 

Results for commit a9a8a79. ± Comparison against base commit 2e0e590.

This pull request removes 32 and adds 192 tests. Note that renamed tests count towards both.
tests.component.system._collections.test_physical_channel_collection ‑ test___physical_channels___getitem_str___shared_interpreter[grpc_init_kwargs-ai_physical_chans]
tests.component.system._collections.test_physical_channel_collection ‑ test___physical_channels___getitem_str___shared_interpreter[grpc_init_kwargs-ao_physical_chans]
tests.component.system._collections.test_physical_channel_collection ‑ test___physical_channels___getitem_str___shared_interpreter[grpc_init_kwargs-ci_physical_chans]
tests.component.system._collections.test_physical_channel_collection ‑ test___physical_channels___getitem_str___shared_interpreter[grpc_init_kwargs-co_physical_chans]
tests.component.system._collections.test_physical_channel_collection ‑ test___physical_channels___getitem_str___shared_interpreter[grpc_init_kwargs-di_lines]
tests.component.system._collections.test_physical_channel_collection ‑ test___physical_channels___getitem_str___shared_interpreter[grpc_init_kwargs-di_ports]
tests.component.system._collections.test_physical_channel_collection ‑ test___physical_channels___getitem_str___shared_interpreter[grpc_init_kwargs-do_lines]
tests.component.system._collections.test_physical_channel_collection ‑ test___physical_channels___getitem_str___shared_interpreter[grpc_init_kwargs-do_ports]
tests.component.system._collections.test_physical_channel_collection ‑ test___physical_channels___getitem_str___shared_interpreter[library_init_kwargs-ai_physical_chans]
tests.component.system._collections.test_physical_channel_collection ‑ test___physical_channels___getitem_str___shared_interpreter[library_init_kwargs-ao_physical_chans]
…
tests.component.system._collections.test_physical_channel_collection ‑ test___physical_channels___getitem_mixed_str_list___name[grpc_init_kwargs-ai_physical_chans]
tests.component.system._collections.test_physical_channel_collection ‑ test___physical_channels___getitem_mixed_str_list___name[grpc_init_kwargs-ao_physical_chans]
tests.component.system._collections.test_physical_channel_collection ‑ test___physical_channels___getitem_mixed_str_list___name[grpc_init_kwargs-ci_physical_chans]
tests.component.system._collections.test_physical_channel_collection ‑ test___physical_channels___getitem_mixed_str_list___name[grpc_init_kwargs-co_physical_chans]
tests.component.system._collections.test_physical_channel_collection ‑ test___physical_channels___getitem_mixed_str_list___name[grpc_init_kwargs-di_lines]
tests.component.system._collections.test_physical_channel_collection ‑ test___physical_channels___getitem_mixed_str_list___name[grpc_init_kwargs-di_ports]
tests.component.system._collections.test_physical_channel_collection ‑ test___physical_channels___getitem_mixed_str_list___name[grpc_init_kwargs-do_lines]
tests.component.system._collections.test_physical_channel_collection ‑ test___physical_channels___getitem_mixed_str_list___name[grpc_init_kwargs-do_ports]
tests.component.system._collections.test_physical_channel_collection ‑ test___physical_channels___getitem_mixed_str_list___name[library_init_kwargs-ai_physical_chans]
tests.component.system._collections.test_physical_channel_collection ‑ test___physical_channels___getitem_mixed_str_list___name[library_init_kwargs-ao_physical_chans]
…

♻️ This comment has been updated with latest results.

@zhindes zhindes requested a review from bkeryan March 17, 2024 20:05
Copy link
Collaborator

@bkeryan bkeryan 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 suggestions

@zhindes zhindes enabled auto-merge (squash) March 22, 2024 20:04
@zhindes zhindes merged commit ef87d63 into master Mar 22, 2024
13 checks passed
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.

Indexing PhysicalChannelCollection fails for slices and strings containing a list/range of channels
2 participants