-
Notifications
You must be signed in to change notification settings - Fork 186
[redcap] add functionality to parse decimal options #10096
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
MaximeBICMTL
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR Adam. This does not break anything for MPN and the code looks reasonable (although I wonder why Q1K selects are different).
Maybe @regisoc has an opinion, but if not that is ok from me.
| if (empty($choice)) { | ||
| continue; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this case ever happen?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it does because look
lorisadmin@adaudrich-dev:/var/www/loris/project/tools$ php redcap2linst.php -o /data/imports/testzone/q1k/redcap -i api -r ooooooooooooooo/ -p 00
-- Getting metadata from REDCap connection.
-- Parsing records...
Warning: Auto-indexing malformed option:
'0.01' as index 1
Warning: Auto-indexing malformed option:
'0.05' as index 2
Warning: Auto-indexing malformed option:
'0.10' as index 3
Warning: Auto-indexing malformed option:
'0.15' as index 4
Warning: Auto-indexing malformed option:
'm' as index 1
Warning: Auto-indexing malformed option:
'n' as index 2
Warning: Auto-indexing malformed option:
'b/p' as index 3
Warning: Auto-indexing malformed option:
'd/t' as index 4
Warning: Auto-indexing malformed option:
'g/k' as index 5
Warning: Auto-indexing malformed option:
'w' as index 6
Warning: Auto-indexing malformed option:
'h' as index 7
Warning: Auto-indexing malformed option:
'f' as index 8
Warning: Auto-indexing malformed option:
'v' as index 9
Warning: Auto-indexing malformed option:
'y' as index 10
Warning: Auto-indexing malformed option:
's/sh' as index 11
-- Writing LINST/META files.
-> writing 'eeget_session_log'
-> writing 'mri_safety_questionnaire'
-> writing 'waisiv_scoring'
-> writing 'mri_acquisition_checklist'
etc...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By "this case", I mean this if (empty($choice)) specifically. It is different than the code change below that prints the warning. Please either provide an example where it is required or remove this if IMO.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@adamdaudrich @MaximeBICMTL Trying to fully understand this case.
Is this only one radio triggering the issue? If so: the idea to normalize on LORIS is good, but as far as I can tell, both metadata and data come from REDCap, so REDCap is the source of truth.
If all other radio contain indices in their declaration (is it the case?), this one must too, and this normalization must happen in REDCap.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think what happened here was the index was 0.01 => 0.01 and redcap2linst couldn't handle the 0.01 as the index
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll take a look at the various tests in which it happened
The REDcap "radio option" sometimes includes no index and, in thtis case, a decimal (float) causing redcap2linst show the following error:
PHP Fatal error: Uncaught DomainException: Could not parse radio option: '0.01 ' in /var/www/loris/project/modules/redcap/php/client/models/redcapdictionaryrecord.class.inc:316 Stack trace: #0 /var/www/loris/project/modules/redcap/php/client/models/redcapdictionaryrecord.class.inc(268): LORIS\redcap\client\models\RedcapDictionaryRecord->_optionsToLINST() #1 /var/www/loris/project/tools/redcap2linst.php(74): LORIS\redcap\client\models\RedcapDictionaryRecord->toLINST() #2 {main} thrown in /var/www/loris/project/modules/redcap/php/client/models/redcapdictionaryrecord.class.inc on line 316
the fetch was 0.01 | 0.05 | 0.10 | 0.15 without indices, But this code fixes it as shown in image 2 and LINST code below
selectmultiple{@}wppsi_47_critic_5{@}Critical Value Significance
Level{@}'1'=>'0.01'{-}'2'=>'0.05'{-}'3'=>'0.10'{-}'4'=>'0.15'`
Relevant documentation is the REDcap importer docs, which could be clarified to address this and other bugs that sometimes occur when interfacing with REDcap instance variances.
Testing instructions (if applicable)
Testing requires a redcap instance with an instrument containing this particular setting.
Link(s) to related issue(s)
This change replicates CBIGR PR #546