Allow arrays of strings to be used with Waveforms #102

AlexanderWells-diamond · 2022-08-25T08:36:55Z

Allow creation records like:

t1 = builder.WaveformOut(
    "TEST1",  initial_value=["ZERO", "ONE", "MANY"]
)
t2 = builder.WaveformOut(
    "TEST2",  initial_value=[b"ZERO", b"ONE", b"MANY"]
)

Where each element of the array is treated as an EPICS string

Values returned will always be Python string lists:

> t1.get()
["ZERO", "ONE", "MANY"]
> t2.get()
["ZERO", "ONE", "MANY"]

(valid) Unicode characters may also be passed in, and will be returned as-is. Note that it is not always obvious how many Unicode characters can be safely entered into EPICS's 40 character string limits.

codecov · 2022-08-25T08:39:32Z

Codecov Report

Merging #102 (65f8bd8) into master (565bca1) will increase coverage by 0.19%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #102      +/-   ##
==========================================
+ Coverage   87.15%   87.34%   +0.19%     
==========================================
  Files          14       14              
  Lines         981      996      +15     
==========================================
+ Hits          855      870      +15     
  Misses        126      126

Impacted Files	Coverage Δ
softioc/builder.py	`97.64% <100.00%> (+0.02%)`	⬆️
softioc/device.py	`95.81% <100.00%> (+0.21%)`	⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

github-actions · 2022-08-25T08:49:17Z

Unit Test Results

    15 files ±    0     15 suites ±0 27m 47s ⏱️ -21s
  271 tests +  22   255 ✔️ +  12   16 💤 +  10 0 ❌ ±0
4 065 runs +330 3 475 ✔️ +180 590 💤 +150 0 ❌ ±0

Results for commit 65f8bd8. ± Comparison against base commit 565bca1.

♻️ This comment has been updated with latest results.

AlexanderWells-diamond · 2022-08-26T07:32:36Z

The current state of the branch is that you can create a WaveformIn/Out with an initial_value of an array of (byte) strings, but you cannot set that value at a later time. This is because if there is no dtype, numpy defaults to float64, and trying to coerce an array of byte strings to float64 is simply nonsense.

If you specify an FTVL or dtype of the relevant type during record creation, the record does behave as expected.

I'm unsure if we can support arrays of strings quite as freely as we could in version 3.2.1 - I don't see a neat way around the implicit numpy conversion.

softioc/builder.py

Note the tests fail, largely due to now numpy defaults to a floating point dtype, which we cannot coerce into an array of strings. Also TODOs need completion...

Note that these tests don't currently pass!

Also make all the tests pass. Involved skipping

Also add more tests

Araneidae

Think there are a few minor things to tidy up, but looks good.

softioc/builder.py

softioc/device.py

softioc/fields.py

tests/test_record_values.py

Araneidae · 2023-03-31T07:34:05Z

docs/reference/api.rst

+    .. note::
+        When storing arrays of strings, it is possible to store Unicode characters.
+        However, as EPICS has no Unicode support the resultant values will be stored
+        as byte strings. Care must be taken when encoding/decoding the values.
+


Don't really think this note captures the essence of this new feature. In particular, the internal encoding is almost completely hidden from users, and it's a bit of a misconception to say that "EPICS has no Unicode support".

We should reference the fact that waveforms of strings are treated as Python Unicode strings throughout and are managed as Python arrays unlike all other waveforms.

Rewritten in 5b59ed7 - how does that sound?

softioc/device.py

- Rewrites the note on waveform of strings - Refactors _require_waveform - Removes unused DTYPE and add more tests for mixed lists

Araneidae

ok

AlexanderWells-diamond linked an issue Aug 25, 2022 that may be closed by this pull request

Waveform of strings no longer works #101

Closed

AlexanderWells-diamond marked this pull request as draft August 25, 2022 08:37

AlexanderWells-diamond force-pushed the 101-waveform-of-strings-no-longer-works branch from 220677d to aa8a8b3 Compare August 25, 2022 12:45

AlexanderWells-diamond requested a review from Araneidae September 7, 2022 12:49

coretl reviewed Sep 16, 2022

View reviewed changes

softioc/builder.py Outdated Show resolved Hide resolved

AlexanderWells-diamond force-pushed the 101-waveform-of-strings-no-longer-works branch from aa8a8b3 to 87e748b Compare October 14, 2022 14:30

AlexanderWells-diamond force-pushed the 101-waveform-of-strings-no-longer-works branch from 87e748b to 3e3d20e Compare March 29, 2023 09:19

AlexanderWells-diamond added 3 commits March 29, 2023 10:21

Allow arrays of strings to be used with Waveforms

7d852a0

Note the tests fail, largely due to now numpy defaults to a floating point dtype, which we cannot coerce into an array of strings. Also TODOs need completion...

Add support and tests for List[str]

f127bf2

Note that these tests don't currently pass!

Handle returning a numpy array of strings

01163d2

Also make all the tests pass. Involved skipping

AlexanderWells-diamond force-pushed the 101-waveform-of-strings-no-longer-works branch from 3e3d20e to ef3204b Compare March 29, 2023 09:22

Add exception on overlong string setting

01308e0

Also add more tests

AlexanderWells-diamond force-pushed the 101-waveform-of-strings-no-longer-works branch from ef3204b to 01308e0 Compare March 29, 2023 09:26

Return a list of strings regardless of input

84346b8

AlexanderWells-diamond marked this pull request as ready for review March 29, 2023 10:41

AlexanderWells-diamond mentioned this pull request Mar 30, 2023

Structure for making a pva table PandABlocks/PandABlocks-ioc#4

Merged

Araneidae requested changes Mar 31, 2023

View reviewed changes

Code review changes

5b59ed7

- Rewrites the note on waveform of strings - Refactors _require_waveform - Removes unused DTYPE and add more tests for mixed lists

Araneidae approved these changes Mar 31, 2023

View reviewed changes

Move CHANGELOG message to correct position

65f8bd8

AlexanderWells-diamond merged commit 65f8bd8 into master Apr 4, 2023

Araneidae deleted the 101-waveform-of-strings-no-longer-works branch May 30, 2023 09:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow arrays of strings to be used with Waveforms #102

Allow arrays of strings to be used with Waveforms #102

AlexanderWells-diamond commented Aug 25, 2022 •

edited

Loading

codecov bot commented Aug 25, 2022 •

edited

Loading

github-actions bot commented Aug 25, 2022 •

edited

Loading

AlexanderWells-diamond commented Aug 26, 2022 •

edited

Loading

Araneidae left a comment

Araneidae Mar 31, 2023

AlexanderWells-diamond Mar 31, 2023

Araneidae left a comment

Allow arrays of strings to be used with Waveforms #102

Allow arrays of strings to be used with Waveforms #102

Conversation

AlexanderWells-diamond commented Aug 25, 2022 • edited Loading

codecov bot commented Aug 25, 2022 • edited Loading

Codecov Report

github-actions bot commented Aug 25, 2022 • edited Loading

Unit Test Results

AlexanderWells-diamond commented Aug 26, 2022 • edited Loading

Araneidae left a comment

Choose a reason for hiding this comment

Araneidae Mar 31, 2023

Choose a reason for hiding this comment

AlexanderWells-diamond Mar 31, 2023

Choose a reason for hiding this comment

Araneidae left a comment

Choose a reason for hiding this comment

AlexanderWells-diamond commented Aug 25, 2022 •

edited

Loading

codecov bot commented Aug 25, 2022 •

edited

Loading

github-actions bot commented Aug 25, 2022 •

edited

Loading

AlexanderWells-diamond commented Aug 26, 2022 •

edited

Loading