Skip to content

Conversation

@mjohanse-emr
Copy link
Contributor

@mjohanse-emr mjohanse-emr commented Jun 23, 2025

What does this Pull Request accomplish?

Allows panel clients to read/write 2D collections of floats using set_value() and get_value(). The most common expected use case is a list of list of floats, but this change supports tuples, sets, and frozen sets as well.

This change includes:

  • A new converter from Collection[Collection[float]] to/from Double2DArray
  • A refactor to _get_best_matching_type() to allow for detecting nested python collections.
  • Unit tests at each level
  • Additional unit tests for the Enum types to make sure I didn't break those.

Why should this Pull Request be merged?

Satisfies the requirements AB#3166107 (at least for now**)

** Caveat: There are discussions going on about what protobuf type to use for 2D collections. I used Double2DArray for this change as a means to prove out that we can detect nested collections in _convert.py. The actual implementation of the converter may change if we decide to use a different protobuf type to pass these 2D collections over the wire.

What testing has been done?

Unit tests, mypy, ni-python-styleguide.

@github-actions
Copy link
Contributor

github-actions bot commented Jun 23, 2025

Test Results

   10 files  ±  0     10 suites  ±0   19s ⏱️ -3s
  189 tests + 39    189 ✅ + 39  0 💤 ±0  0 ❌ ±0 
1 840 runs  +370  1 840 ✅ +370  0 💤 ±0  0 ❌ ±0 

Results for commit f06cdb9. ± Comparison against base commit dcaf336.

This pull request removes 5 and adds 44 tests. Note that renamed tests count towards both.
tests.unit.test_convert ‑ test___various_python_objects___get_best_matching_type___returns_correct_type_string[python_object5-Collection.bool]
tests.unit.test_convert ‑ test___various_python_objects___get_best_matching_type___returns_correct_type_string[python_object6-Collection.bytes]
tests.unit.test_convert ‑ test___various_python_objects___get_best_matching_type___returns_correct_type_string[python_object7-Collection.float]
tests.unit.test_convert ‑ test___various_python_objects___get_best_matching_type___returns_correct_type_string[python_object8-Collection.int]
tests.unit.test_convert ‑ test___various_python_objects___get_best_matching_type___returns_correct_type_string[python_object9-Collection.str]
tests.unit.test_convert ‑ test___double2darray___from_any___valid_python_2dcollection
tests.unit.test_convert ‑ test___python_2dcollection_of_float___to_any___valid_double2darray[python_value0]
tests.unit.test_convert ‑ test___python_2dcollection_of_float___to_any___valid_double2darray[python_value1]
tests.unit.test_convert ‑ test___python_2dcollection_of_float___to_any___valid_double2darray[python_value2]
tests.unit.test_convert ‑ test___python_2dcollection_of_float___to_any___valid_double2darray[python_value3]
tests.unit.test_convert ‑ test___python_2dcollection_of_float___to_any___valid_double2darray[python_value4]
tests.unit.test_convert ‑ test___python_2dcollection_of_float___to_any___valid_double2darray[python_value5]
tests.unit.test_convert ‑ test___python_2dcollection_of_float___to_any___valid_double2darray[python_value6]
tests.unit.test_convert ‑ test___python_2dcollection_of_float___to_any___valid_double2darray[python_value7]
tests.unit.test_convert ‑ test___python_set_of_collection_of_float___to_any___valid_double2darray[python_value0]
…

♻️ This comment has been updated with latest results.

@mjohanse-emr mjohanse-emr marked this pull request as ready for review June 23, 2025 18:19
@mjohanse-emr mjohanse-emr requested review from bkeryan and jfriedri-ni and removed request for csjall June 23, 2025 18:19
@mikeprosserni
Copy link
Collaborator

Can you add the new 2D types to the new all_types example? You can just add them to all_types_with_values in examples\all_types\define_types.py.

Signed-off-by: Michael Johansen <[email protected]>
@mjohanse-emr mjohanse-emr merged commit 3cabf0c into main Jun 25, 2025
14 checks passed
@mjohanse-emr mjohanse-emr deleted the users/mjohanse/2d_type_inspection branch June 25, 2025 16:19
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