Standardise record value setting/getting

[AlexanderWells-diamond]

This PR changes the handling of types for all records, on both setting and retrieval. Broadly, records are now more picky about what types they will and will not accept, and exceptions will generally be raised if invalid values are put to a record.

This PR also introduces two new records, longStringIn and longStringOut, which are specialized versions of Waveform records designed to handle Python Unicode strings. See documentation for more details.

A complete list of valid types for each record is most easily found from looking at the tests - see record_values_list for the list of cases which are tested.

Closes #39
Closes #40
Closes #43
Closes #55

[codecov]

Codecov Report

Merging #60 (0dd1544) into master (3866562) will increase coverage by 3.09%.
The diff coverage is 98.00%.

@@            Coverage Diff             @@
##           master      #60      +/-   ##
==========================================
+ Coverage   85.39%   88.49%   +3.09%     
==========================================
  Files          13       13              
  Lines         815      904      +89     
==========================================
+ Hits          696      800     +104     
+ Misses        119      104      -15     

Impacted Files	Coverage Δ
softioc/imports.py	100.00% <ø> (ø)
softioc/builder.py	96.89% <93.33%> (+0.98%)	⬆️
softioc/device.py	98.73% <99.25%> (+10.59%)	⬆️
softioc/__init__.py	92.30% <100.00%> (ø)
softioc/fields.py	86.11% <100.00%> (-5.56%)	⬇️
softioc/pythonSoftIoc.py	90.00% <100.00%> (+0.34%)	⬆️
softioc/softioc.py	87.85% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3866562...0dd1544. Read the comment docs.

[AlexanderWells-diamond]

PR parked pending removal of Python2 support:
#61

This will mean we won't need to handle Unicode in Python2.

[AlexanderWells-diamond]

@Araneidae @thomascobb This PR is now ready for review.

[Araneidae]

Most of my comments so far are layout related.

I'm getting a bit distracted by inconsistent layout and use of spaces around =. I'm guessing that spaces in function definitions is a lost call, but you'll find that all the preexisting code always uses spaces in all other contexts, particularly in function calls.

Secondly, please use 4 character indent except in those incredibly rare cases where it comes out to be particularly ugly. You can always use a doc-string to make function argument indentation to look sensible.

I'd like the record device definitions to look more uniform and be easier to read, possibly even drop all(?) defaults? Anyhow, try and get a layout that's easier on the eye here.

There's plenty of functionality for me to look at ... I'll get to it ...

[Araneidae]

Ok

[AlexanderWells-diamond]

The latest additions to the testware bring the number of tests up to ~500. This takes 5 minutes on my machine, and up to 15 minutes on a CI runner. This is getting to the stage where it's too long. There's also now non-trivial amounts of test code, to account for the wide variety of situations encountered (pythonSoftIOC issues, differing behaviour between In and Out records, differing behaviour of Waveforms, many individual tests skipped, etc.).

I'll look at refactoring these tests into larger tests that should hopefully run quicker (but not needing to spawn a new multiprocess.process for each test. This will probably take the form of having large numbers of records created per process. This'll likely lose resolution, as tests will likely stop on first failure rather than listing all failures.

[github-actions]

Unit Test Results

     12 files       12 suites   10m 1s ⏱️
   186 tests    179 ✔️     7 💤 0 ❌
2 224 runs  1 984 ✔️ 240 💤 0 ❌

Results for commit 0dd1544.

♻️ This comment has been updated with latest results.

[Araneidae]

This is looking very good.

I did notice a number of new lines not covered by tests. Given how comprehensive the tests already are now, I wonder if it would be worth adding a few more tests to cover these lines.

Finally, the documentation comment on byte strings makes me wonder whether we should have special support for bytes? I think this is in just one place in the code:

pythonSoftIOC/softioc/device.py

Lines 322 to 325 in 98de8f6

	def _require_waveform(value, dtype):
	if isinstance(value, bytes):
	# Special case hack for byte arrays. Surprisingly tricky:
	value = numpy.frombuffer(value, dtype = numpy.uint8)

--- I'm ever so tempted to simply drop this special case!

[Araneidae]

This strong tie to a very particular version of epicscorelibs rings an alarm bell for me. What's going on here? Surely we don't want this, do we?

Separately, the version number is in an alarmingly non-standard format!

[thomascobb]

This strong tie to a very particular version of epicscorelibs rings an alarm bell for me. What's going on here?

There is an ordering issue here. The build process is this:

• deps from pyproject.toml are installed
• python setup.py bdist_wheel is called to make a binary wheel, which is compiled against the exact version of epicscorelibs that has been installed, and records this fact in the wheel
• the wheel is uploaded to PyPI
• anyone using this version of the binary wheel must have this exact version of epicscorelibs installed too (which pip will do automatically)
• anyone using the sdist is free to have whatever version of epicscorelibs they want

The >= that was there originally was correct, which would mean that pythonSoftIOC was compiled against the latest available epicscorelibs. Unfortunately in the tests we require p4p, which is also compiled against a particular version of epicscorelibs. If a new version of epicscorelibs is released, but p4p is not, then pip will fail to install dependencies. This is why we are using ==. Without ABI compliance for EPICS, I can't see a way around this, unless a release of p4p is made with every epicscorelibs release. Fortunately we don't care for aioca as we only need API compliance, not ABI compliance.

@mdavidsaver can you think of a way we can resolve this?

Separately, the version number is in an alarmingly non-standard format!

The first part is the EPICS release, but not sure about the other bits. Maybe @mdavidsaver could comment on this too?

[mdavidsaver]

If a new version of epicscorelibs is released, but p4p is not

Sorry. I've been sloppy about this as I'm in the (so far long) process of introducing another dependency pvxslibs. I'll try to straighten this out. Though even if I do, there will always be some time when pypy.org has newer versions of parts of the dependency chain. Maybe only minutes, but never zero.

we only need API compliance, not ABI compliance.

Speaking strictly, API is important at build time (with a compiler), and ABI afterwards. So in an ideal world, pyproject.toml is expressing the build time dependency (BuildRequires in an RPM .spec file), while install_requires in setup.py expresses the runtime dependency (plain Requires in a .spec). As with a .spec runtime dependencies are partly generated. so...

pythonSoftIOC/setup.py

Lines 108 to 109 in 3866562

	# Dependency version declared in pyproject.toml
	epicscorelibs.version.abi_requires(),

So if epicscorelibs 7.0.6.99.1.0 is installed, this will expand to:

epicscorelibs >=7.0.6.99.1.0, <7.0.6.99.2

Separately, the version number is in an alarmingly non-standard format!

There are three pieces of information being encoded: upstream EPICS Base version (everything up to and including the .99), epicscorelibs ABI, and epicscorelibs revision. So the version range being emitted above allows for later revisions which (hopefully) don't include any ABI change.

[mdavidsaver]

@mdavidsaver can you think of a way we can resolve this?

If the problem is only with GHA, you might be able to pass --no-binary p4p to pip when installing the dev dependencies prior to running tests.

[AlexanderWells-diamond]

Thanks @mdavidsaver it looks like p4p 3.5.5 fixes the issue - I've restored the original epicscorelibs dependency in this commit and it seems to be working: adaa286

Spoke too soon, the Code CI works but the Docs CI breaks with this change... Investigating...

[AlexanderWells-diamond]

I did notice a number of new lines not covered by tests. Given how comprehensive the tests already are now, I wonder if it would be worth adding a few more tests to cover these lines.

@Araneidae Much of the missed code was actually being run already, it just wasn't being picked up by Codecov due to it happening in a multiprocessing.Process rather than on the main thread. I've added code in dd99311 to now count it. There's only a handful of missed lines now, at least in device.py.

[Araneidae]

@AlexanderWells-diamond , I'm happy for you to merge this when you're ready now.