Skip to content

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

Sign up for GitHub

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

[redcap] add functionality to parse decimal options #10096

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a0a7a71
add functionality to parse decimal options
adamdaudrich Nov 7, 2025
ec53c55
made static error corrections
adamdaudrich Nov 7, 2025
5bd3950
fixed typo on line 334
adamdaudrich Nov 8, 2025
280fd54
spaces before equal sign
adamdaudrich Nov 8, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 22 additions & 6 deletions modules/redcap/php/client/models/redcapdictionaryrecord.class.inc
View file
Open in desktop
Copy link
Contributor

@regisoc regisoc Nov 10, 2025

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.

Copy link
Collaborator Author

@adamdaudrich adamdaudrich Nov 10, 2025

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

Copy link
Collaborator Author

@adamdaudrich adamdaudrich Nov 10, 2025

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

Original file line number Diff line number Diff line change
Expand Up @@ -313,17 +313,33 @@ class RedcapDictionaryRecord

$choices = explode('|', $dictionary);
$linstChoices = [];
$pattern = "/^(\s)*([[:alnum:]]+)(\s)*,(.*)$/";
$pattern = "/^(\s)*([[:alnum:]._]+)(\s)*,(.*)$/";

$index = 1; //Auto-index counter
foreach ($choices as $choice) {
$choice = trim($choice);
if (empty($choice)) {
continue;
}
Comment on lines +321 to +323
Copy link
Contributor

@MaximeBICMTL MaximeBICMTL Nov 10, 2025

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?

Copy link
Collaborator Author

@adamdaudrich adamdaudrich Nov 10, 2025

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

Copy link
Contributor

@MaximeBICMTL MaximeBICMTL Nov 11, 2025 •
edited
Loading

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.


$matches = [];

if (preg_match($pattern, $choice, $matches) !== 1) {
throw new \DomainException(
"Could not parse radio option: '$choice'"
// Missing code -auto-generate one
error_log(
"Warning: Auto-indexing malformed option:
'$choice' as index $index"
);

$backend = $index;
$label = $choice;
$index++;
} else {
//Normal parsing
$backend = $matches[2];
$label = trim($matches[4]);
}
$backend = $matches[2];
$linstFormat = "'$backend'=>'" . trim($matches[4]) . '\'';

$linstFormat = "'$backend'=>'$label'";
$linstChoices[] = $linstFormat;

}
Expand Down
Loading