Fix error handling when unsupported (and required) extensions are read

[pvillacorta]

No description provided.

[codecov]

Codecov Report

❌ Patch coverage is 87.50000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 89.07%. Comparing base (21a7c25) to head (7aa750e).
⚠️ Report is 7 commits behind head on master.

Files with missing lines	Patch %	Lines
KomaMRIFiles/src/Sequence/Pulseq.jl	87.50%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #729      +/-   ##
==========================================
- Coverage   91.03%   89.07%   -1.97%     
==========================================
  Files          61       61              
  Lines        3357     3358       +1     
==========================================
- Hits         3056     2991      -65     
- Misses        301      367      +66     

Flag	Coverage Δ
base	90.86% <ø> (ø)
core	77.44% <ø> (-12.30%)	⬇️
files	95.03% <87.50%> (+0.47%)	⬆️
komamri	88.13% <ø> (ø)
plots	91.00% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
KomaMRIFiles/src/Sequence/Pulseq.jl	94.15% <87.50%> (+0.64%)	⬆️

... and 8 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[pvillacorta]

We need to add a sequence such as that in #728 for testing (how can we test when an error is the expected output?)

[cncastillo]

why is this change needed? I dont think the error comes from not identifying the dictionary key "RequiredExtensions" unless in the code it means something else. I can fully remove the error by defining rotations. I will upload a PR, so you can see what i mean.

[cncastillo]

#730

[pvillacorta]

This change is needed for correct error handling when reading unknown extensions. There are no tests for these cases yet (I can add one in this PR). Handling was not done correctly in #614 due to two main reasons:

1. Required extensions were read as an array of strings in case several extensions are present, and as a single string in case there is only one. The sequence from #728 was an example of the latter; that is why this error use occursin(needle, haystack) for string containment was appearing. Now, both cases are are read as arrays of strings.
2. Specifications of an unknown extension were not skipped, so each line of, let's say, this fragment:

extension ROTATIONS 1
1  0.474662 -0.736136 0.405504 -0.26147
2  0.301665 0.665882 -0.621542 -0.281578
3  0.543231 0.0806243 -0.122688 -0.826648
4  0.172965 -0.197188 0.725449 -0.636336
5  0.25095 0.879218 -0.389417 -0.111149
6  0.743181 0.225569 -0.182951 -0.602768
7  0.46415 -0.393656 0.513232 -0.605139
8  0.048341 0.367781 -0.920736 -0.121021
9  0.859788 0.406897 -0.131986 -0.27889
10  0.696252 -0.432705 0.302304 -0.486428
11  0.0110341 -0.66263 0.748762 -0.0124684
12  0.377668 0.252521 -0.495158 -0.740553
13  0.905436 -0.392155 0.0645757 -0.149097
14  0.140515 -0.84915 0.502286 -0.0831169
15  0.527918 0.478855 -0.471254 -0.519538
16  0.506777 -0.135563 0.220002 -0.822435
17  0.0630765 -0.196014 0.931527 -0.299762
18  0.570485 0.695807 -0.337432 -0.276658
19  0.75454 -0.0864226 0.0740266 -0.646313
20  0.259684 -0.525894 0.726224 -0.358607
21  0.171279 0.286118 -0.8089 -0.484232
22  1 0 0 -1.22465e-16
...

were read but not detected as anything known, leading to the following message (last else statement from read_seq function): error "Unknown section code: $section".

This PR is thus independent of #730 and equally necessary.

[cncastillo]

Ok, I missed that in the Pulseq specification RequiredExtentions is actually a special key.

Is this PR able to read a list of these? In the way that is currently implemented in this PR, it seems a bit hardcoded:

def[key] = (length(parsed_array) == 1 && key != "RequiredExtensions") ? parsed_array[1] : parsed_array

Why do you need this?

I don't see how the current version of the code matches the behavior in the Pulseq specification.

[pvillacorta]

Fixed.
The code already matched the specification: if it detects an unknown extensions, it just throws a warning:
"Ignoring unsupported extension: $ext_string",
unless that extension is explicitly required; in that case, it throws an error:
"Extension $ext_string is required by the sequence (RequiredExtensions: $required_extensions but not supported by KomaMRI reader"
and stops reading the sequence, as stated within the specification.

[cncastillo]

Could you bump the necessary packages, merge and register new versions?