Add support for builtin types for get_value and set_value #26

jfriedri-ni · 2025-05-14T17:17:15Z

What does this Pull Request accomplish?

This PR implements getting and setting panel values that use builtin Python scalar-like types

int
string
float
bool
bytes

With partial support for

enum.IntFlag
enum.IntEnum
enum.StrEnum

And a baseline extensible converter collection.

Why should this Pull Request be merged?

This is core functionality for the package.

What testing has been done?

New tests

test___unopened_panel___set_value___sets_value
test___unopened_panel___get_unset_value___raises_exception
test___unopened_panel___get_set_value___gets_value
test___builtin_scalar_types___set_value___gets_same_value

Hand test

Run placeholder.py with a genuine NI.PythonPanel.V1.Service.exe running

Signed-off-by: Joe Friedrichsen <114173023+jfriedri-ni@users.noreply.github.com>

…med tuple Signed-off-by: Joe Friedrichsen <114173023+jfriedri-ni@users.noreply.github.com>

Signed-off-by: Joe Friedrichsen <114173023+jfriedri-ni@users.noreply.github.com>

…-get-set-panel-value

github-actions · 2025-05-14T17:19:28Z

Test Results

10 files ± 0 10 suites ±0 6s ⏱️ -1s
24 tests + 18 24 ✅ + 18 0 💤 ±0 0 ❌ ±0
210 runs +150 210 ✅ +150 0 💤 ±0 0 ❌ ±0

Results for commit 83e6363. ± Comparison against base commit 5b1cef0.

♻️ This comment has been updated with latest results.

jfriedri-ni

Here are some of the larger tradeoffs and decisions we need to make.

examples/placeholder.py

src/nipanel/_converters.py

tests/unit/test_streamlit_panel.py

src/nipanel/_converters.py

examples/placeholder.py

src/nipanel/_converters.py

tests/unit/test_streamlit_panel.py

examples/placeholder.py

src/nipanel/_converters.py

tests/unit/test_streamlit_panel.py

examples/placeholder.py

src/nipanel/_converters.py

tests/unit/test_streamlit_panel.py

Signed-off-by: Joe Friedrichsen <114173023+jfriedri-ni@users.noreply.github.com>

…hannel' Signed-off-by: Joe Friedrichsen <114173023+jfriedri-ni@users.noreply.github.com>

Signed-off-by: Joe Friedrichsen <114173023+jfriedri-ni@users.noreply.github.com>

tests/conftest.py

src/nipanel/_converters.py

Signed-off-by: Joe Friedrichsen <114173023+jfriedri-ni@users.noreply.github.com>

…can access DESCRIPTOR Signed-off-by: Joe Friedrichsen <114173023+jfriedri-ni@users.noreply.github.com>

jfriedri-ni

Assuming we want a Protocol or an ABC, I'm unsure how to correctly appease the type checker given how the code uses the converters.

src/nipanel/_converters.py

tests/conftest.py

tests/unit/test_streamlit_panel.py

Signed-off-by: Joe Friedrichsen <114173023+jfriedri-ni@users.noreply.github.com>

… is too difficult (for me) Signed-off-by: Joe Friedrichsen <114173023+jfriedri-ni@users.noreply.github.com>

Signed-off-by: Joe Friedrichsen <114173023+jfriedri-ni@users.noreply.github.com>

…ts_value Signed-off-by: Joe Friedrichsen <114173023+jfriedri-ni@users.noreply.github.com>

…-get-set-panel-value

src/nipanel/_converters.py

Signed-off-by: Joe Friedrichsen <114173023+jfriedri-ni@users.noreply.github.com>

src/nipanel/_converters.py

Signed-off-by: Brad Keryan <brad.keryan@ni.com>

src/nipanel/_converters.py

jfriedri-ni added 7 commits May 13, 2025 16:52

First draft of builtin type marshalling

6643e6b

Signed-off-by: Joe Friedrichsen <114173023+jfriedri-ni@users.noreply.github.com>

Use a brute-force explicit helper for type-hinting the convertible na…

8d97456

…med tuple Signed-off-by: Joe Friedrichsen <114173023+jfriedri-ni@users.noreply.github.com>

Use the StreamlitPanel as the main entrypoint

02c1cdd

Signed-off-by: Joe Friedrichsen <114173023+jfriedri-ni@users.noreply.github.com>

Add tests for unopened-set, unopened-get, and scalar round trips

ec9e760

Signed-off-by: Joe Friedrichsen <114173023+jfriedri-ni@users.noreply.github.com>

Remove unused import

4407668

Signed-off-by: Joe Friedrichsen <114173023+jfriedri-ni@users.noreply.github.com>

Add test showing an unset value raises when clients attempt to get it

86c22aa

Signed-off-by: Joe Friedrichsen <114173023+jfriedri-ni@users.noreply.github.com>

Merge remote-tracking branch 'origin/main' into users/jfriedri-ni/add…

b3e504a

…-get-set-panel-value

jfriedri-ni requested review from csjall and mikeprosserni as code owners May 14, 2025 17:17

jfriedri-ni commented May 14, 2025

View reviewed changes

jfriedri-ni requested a review from bkeryan May 14, 2025 17:34

jfriedri-ni commented May 14, 2025

View reviewed changes

src/nipanel/_converters.py Outdated Show resolved Hide resolved

bkeryan requested changes May 14, 2025

View reviewed changes

csjall reviewed May 14, 2025

View reviewed changes

jfriedri-ni added 11 commits May 15, 2025 11:53

Add type hints that are compatible with Python 3.9

b63ea73

Signed-off-by: Joe Friedrichsen <114173023+jfriedri-ni@users.noreply.github.com>

Do not add type hints when the analyzer can infer it

a60c17f

Signed-off-by: Joe Friedrichsen <114173023+jfriedri-ni@users.noreply.github.com>

Parameterize the scalar value test

b3f1314

Signed-off-by: Joe Friedrichsen <114173023+jfriedri-ni@users.noreply.github.com>

Add roundtrip tests for IntFlag, IntEnum, StrEnum

4d73ca6

Signed-off-by: Joe Friedrichsen <114173023+jfriedri-ni@users.noreply.github.com>

Add tests for mixin-enums and bare-enums

518dd2f

Signed-off-by: Joe Friedrichsen <114173023+jfriedri-ni@users.noreply.github.com>

Shorten typename imports

345e89a

Signed-off-by: Joe Friedrichsen <114173023+jfriedri-ni@users.noreply.github.com>

Rename fixture 'grpc_channel_for_fake_panel_service' to 'fake_panel_c…

3a02372

…hannel' Signed-off-by: Joe Friedrichsen <114173023+jfriedri-ni@users.noreply.github.com>

Use grcpio's logging thread pool

ca9b8a3

Signed-off-by: Joe Friedrichsen <114173023+jfriedri-ni@users.noreply.github.com>

Remove static storage for the fake servicer

91ee96b

Signed-off-by: Joe Friedrichsen <114173023+jfriedri-ni@users.noreply.github.com>

Remove class members from the fake service

63296e1

Signed-off-by: Joe Friedrichsen <114173023+jfriedri-ni@users.noreply.github.com>

Simplify class and storage for ConvertibleType

5969e92

Signed-off-by: Joe Friedrichsen <114173023+jfriedri-ni@users.noreply.github.com>

bkeryan reviewed May 15, 2025

View reviewed changes

tests/conftest.py Outdated Show resolved Hide resolved

bkeryan reviewed May 15, 2025

View reviewed changes

src/nipanel/_converters.py Outdated Show resolved Hide resolved

jfriedri-ni added 2 commits May 15, 2025 15:17

Use pyproject.toml to suppress type checker

3fb2b9b

Signed-off-by: Joe Friedrichsen <114173023+jfriedri-ni@users.noreply.github.com>

Clarify docstring

62f808c

Signed-off-by: Joe Friedrichsen <114173023+jfriedri-ni@users.noreply.github.com>

Make protobuf_message return a Message so the default implementation …

3285505

…can access DESCRIPTOR Signed-off-by: Joe Friedrichsen <114173023+jfriedri-ni@users.noreply.github.com>

jfriedri-ni commented May 16, 2025

View reviewed changes

src/nipanel/_converters.py Outdated Show resolved Hide resolved

src/nipanel/_converters.py Outdated Show resolved Hide resolved

src/nipanel/_converters.py Outdated Show resolved Hide resolved

bkeryan reviewed May 16, 2025

View reviewed changes

src/nipanel/_converters.py Outdated Show resolved Hide resolved

bkeryan reviewed May 16, 2025

View reviewed changes

src/nipanel/_converters.py Outdated Show resolved Hide resolved

mikeprosserni requested changes May 19, 2025

View reviewed changes

tests/conftest.py Show resolved Hide resolved

tests/unit/test_streamlit_panel.py Show resolved Hide resolved

jfriedri-ni added 8 commits May 19, 2025 12:09

Simple naive type-fix

e9fe18c

Signed-off-by: Joe Friedrichsen <114173023+jfriedri-ni@users.noreply.github.com>

Refactor converters as an ABC family

dc0c552

Signed-off-by: Joe Friedrichsen <114173023+jfriedri-ni@users.noreply.github.com>

Converting builtins with a shared function, still mypy errors

4f3a208

Signed-off-by: Joe Friedrichsen <114173023+jfriedri-ni@users.noreply.github.com>

Do not share code between the builtin converters because type hinting…

9115100

… is too difficult (for me) Signed-off-by: Joe Friedrichsen <114173023+jfriedri-ni@users.noreply.github.com>

Address the linters

6d8318b

Signed-off-by: Joe Friedrichsen <114173023+jfriedri-ni@users.noreply.github.com>

Follow naming convention for covariant typevars

3313691

Signed-off-by: Joe Friedrichsen <114173023+jfriedri-ni@users.noreply.github.com>

Rely on set_value throwing for test___unopened_panel___set_value___se…

c1324ae

…ts_value Signed-off-by: Joe Friedrichsen <114173023+jfriedri-ni@users.noreply.github.com>

Merge remote-tracking branch 'origin/main' into users/jfriedri-ni/add…

da0a6d7

…-get-set-panel-value

jfriedri-ni requested review from bkeryan and mikeprosserni May 20, 2025 19:40

mikeprosserni approved these changes May 20, 2025

View reviewed changes

bkeryan added 2 commits May 20, 2025 17:29

converters: Refactor message->Any conversion

d5d4c51

converters: Refactor to_python any handling

ae54c82

bkeryan reviewed May 20, 2025

View reviewed changes

src/nipanel/_converters.py Outdated Show resolved Hide resolved

jfriedri-ni added 2 commits May 21, 2025 10:29

Clarify some docstrings

6ff26ea

Signed-off-by: Joe Friedrichsen <114173023+jfriedri-ni@users.noreply.github.com>

Use object in the to_any() and from_any() signatures

072a3ac

Signed-off-by: Joe Friedrichsen <114173023+jfriedri-ni@users.noreply.github.com>

jfriedri-ni requested a review from bkeryan May 21, 2025 17:00

jfriedri-ni changed the title ~~Preview and RFC for supporting builtin types for get_value and set_value~~ Add support for builtin types for get_value and set_value May 21, 2025

bkeryan reviewed May 21, 2025

View reviewed changes

src/nipanel/_converters.py Outdated Show resolved Hide resolved

converters: Use type variables more consistently

83e6363

Signed-off-by: Brad Keryan <brad.keryan@ni.com>

bkeryan approved these changes May 21, 2025

View reviewed changes

jfriedri-ni commented May 21, 2025

View reviewed changes

src/nipanel/_converters.py Show resolved Hide resolved

jfriedri-ni merged commit a87939a into main May 21, 2025
14 checks passed

jfriedri-ni deleted the users/jfriedri-ni/add-get-set-panel-value branch May 22, 2025 16:00

jfriedri-ni mentioned this pull request May 27, 2025

Add a panel accessor class for streamlit scripts to call #31

Merged

Uh oh!

Add support for builtin types for get_value and set_value #26

Add support for builtin types for get_value and set_value #26

Uh oh!

Conversation

jfriedri-ni commented May 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does this Pull Request accomplish?

Why should this Pull Request be merged?

What testing has been done?

Uh oh!

github-actions bot commented May 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test Results

Uh oh!

jfriedri-ni left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jfriedri-ni left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

jfriedri-ni commented May 14, 2025 •

edited

Loading

github-actions bot commented May 14, 2025 •

edited

Loading