BUG: Add a case in p4p_plugin_component.py for Numpy integers. #1120
Conversation
|
looks good, thanks! can we also add a test for passing in the np.int32, in this test-case probably: https://github.com/slaclab/pydm/blob/master/pydm/tests/data_plugins/test_p4p_plugin_component.py#L56 i think the test will need to be modified a bit since we can't wrap the np.int32 in a 'NTScalar'? |
|
@nstelter-slac I still intend to add a test, the last week has just been very busy. I'll try to do that by the end of the week. |
|
@nstelter-slac This seemed to work, though I admit I'm a little out of my depth here. Not sure why some of the CI runs failed. I didn't see an option for me to try re-running them. |
|
Looks like someone kicked the tests, and they passed. Thanks! |
| @@ -51,6 +51,7 @@ def generate_control_variables(value): | |||
| 1, | |||
| ), | |||
| (NTEnum().wrap({"index": 0, "choices": ["YES", "NO", "MAYBE"]}), False, 0, 2), | |||
| (NTScalar("i").wrap({"value": np.int32(10)}), False, np.int32(10), 1), | |||
There was a problem hiding this comment.
it seems like NTScalar.wrap converts the np.int32 into just a python int and it gets passed into the testcase, which is b4 the testcase can try to test it. which also means the if on line 82 is not called, (u can add prints to see this and run pytest with -s arg to see print output in terminal)
if we change line 60 to not require a Value type, by having just
value_to_send,
we can actually pass the raw np.int32 into the testcase, but then the testcase fails since send_new_value function expects a Value type (which doesn't seem to support having a wrapped np.integer type, it just converts to python int)
im curious to debug what happens when using this fix with the real pydm screen, and why it doesnt hit this issue? Do you have a screen calling a PV using an np.integer we could use to debug?
There was a problem hiding this comment.
also so sorry for long delay on this review!
Adds an additional case in the p4p_plugin_component module for Numpy integers.
Tested using an in-development GUI on the same PV described in #1119
Closes #1119