BF: fix trial data writing when there are multiple key presses including None

[mh105]

When there are multiple key presses in a trial, data from all trials are stored as a list of lists. However, for numerical data that underwent replacing 'None' with '', this creates invalid syntaxes when converting strVersion back to a tuple using eval().

This is because eval("[1, , 2]"), etc. is not a valid syntax. Current PR fixes the issue to allow an empty cell to be written into the output file for trials containing data in the form of lists of lists.

[codecov]

Codecov Report

Attention: Patch coverage is 30.00000% with 7 lines in your changes missing coverage. Please review.

Project coverage is 41.15%. Comparing base (e110f5f) to head (9b73829).
Report is 22 commits behind head on dev.

Additional details and impacted files

@@            Coverage Diff             @@
##              dev    #6718      +/-   ##
==========================================
- Coverage   48.96%   41.15%   -7.81%     
==========================================
  Files         332      332              
  Lines       61181    61188       +7     
==========================================
- Hits        29956    25183    -4773     
- Misses      31225    36005    +4780     

Components	Coverage Δ
app	∅ <ø> (∅)
boilerplate	∅ <ø> (∅)
library	∅ <ø> (∅)
vm-safe library	∅ <ø> (∅)

[TEParsons]

I think this is fine - if I follow: You're keeping None present up to the point of the eval call, then removing it after so that behaviour after is unchanged from current?

I'd also advise adding a test to psychopy.tests.test_data.test_TrialHandler:TestTrialHandler to check this - we actually have one called test_multiKeyResponses that's been disabled for about 10 years so it would be good to have something more up to date :')

[TEParsons]

What's the motivation for removing the []? Is it possible for .dtype.kind to be just part of the string "SaUV", and if so do we want to catch it here?

[mh105]

Yeah I don't think 'SaUV' itself is a valid dtype.kind. I think it has always meant to check whether .dtype.kind is part of the string, and it was a typo to include the []. Right now even for string data, 'None' gets replaced by '' because tmpData.dtype.kind is never the same as 'SaUV'.

[TEParsons]

Ah okay, I don't know much about dtypes tbh so I'll take your word on it. To be clearest we should probably do ("S", "a", "U", "V"), but this is a minor point, the string is fine :P

[mh105]

Might be worth checking with Jon or whoever wrote it in the first place? I don't think "a" is one of the valid character codes anymore: https://numpy.org/doc/stable/reference/generated/numpy.dtype.kind.html

[mh105]

I think this is fine - if I follow: You're keeping None present up to the point of the eval call, then removing it after so that behaviour after is unchanged from current?

Yeah that's right. I'm adding the 'None' back into strVersion so that eval() does not crash the script due to invalid syntax. And then after eval() I remove them again if they were removed previously. This way the output file behavior is the same as how it is right now. So this BF really just fixes a bug without changing any behavior.

I'd also advise adding a test to psychopy.tests.test_data.test_TrialHandler:TestTrialHandler to check this - we actually have one called test_multiKeyResponses that's been disabled for about 10 years so it would be good to have something more up to date :')

I agree, although I still don't know well enough why this issue came up. The strange thing is that I ran into this problem by spamming a key when a response_key component is active (and it's set to store all keys). Sometimes, but not always, when I press too quickly, one of the durations becomes None. I still couldn't quite figure out why it would be None...