BUG: Add a case in p4p_plugin_component.py for Numpy integers.

[slactjohnson]

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

[nstelter-slac]

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'?

[slactjohnson]

@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.

[slactjohnson]

@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.

[slactjohnson]

Looks like someone kicked the tests, and they passed. Thanks!
Waiting for an approval of the newly added test prior to merging.

[nstelter-slac]

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?

[nstelter-slac]

also so sorry for long delay on this review!

[slactjohnson]

@nstelter-slac Sorry for the long (long) delay in my reply. I had a long vacation through December, but I'm spinning back up now.

I think my particular use case is described in the issue related to this PR, but I'm happy to help debug further if I can. I haven't seen this occur anywhere else. An example of how I'm trying to index out values from an NTTable is here: https://github.com/pcdshub/opcpa-tpr-config/blob/master/opcpa_tpr_config/widgets.py#L161-L166

At this point I'm a little confused about what to do with the test case; how would you like me to proceed?

[nstelter-slac]

hello!

ok i stepped thru the code with example NTTable and i understand things now

i propose we make 2 minor changes and then merge this in right away! (will get included in upcoming pydm release)

1. support numpy float arrays in NTTable by adding the following to p4p_plugin_component.py:

elif isinstance(new_value, np.float32):
    self.new_value_signal[float].emit(float(new_value))
elif isinstance(new_value, np.float64):
    self.new_value_signal[float].emit(float(new_value))

2. to hit the fix in a test we'd need testcase with NTTable that has numpy int array in it. And i think that needs to use NTTable().wrap to do, but i tried this and had trouble getting it to work as i wanted.

   so for sake of merging i say we dont have a test rn and instead:

   remove the check here thats never true (b/c of the NTScalar.wrap conversion b4) https://github.com/slaclab/pydm/pull/1120/files#diff-ddc8af515570b1d6723e90ebcb5f52bb38c529a4503d039f49b6e7c542b1ac95R82

   and then add a comment like "# can add cases for testing NTTable containing np.integer and np float types arrays here" : at the top of the test function here https://github.com/slaclab/pydm/pull/1120/files#diff-ddc8af515570b1d6723e90ebcb5f52bb38c529a4503d039f49b6e7c542b1ac95R54

again sry this patch got lost in the cracks!

[slactjohnson]

Thanks @nstelter-slac. I've implemented the changes you requested, and the tests passed.