Skip to content

#808 xASL_imp_NII2BIDS_Run: catch errors #809

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

Jump to bottom
Merged
MichaelStritt merged 4 commits into from
Sep 1, 2021
Merged

#808 xASL_imp_NII2BIDS_Run: catch errors #809

MichaelStritt merged 4 commits into from
Sep 1, 2021

Conversation

@HenkMutsaerts
Copy link
Member

@HenkMutsaerts HenkMutsaerts commented Aug 31, 2021

Linked issue

Closes #808

@HenkMutsaerts HenkMutsaerts linked an issue Aug 31, 2021 that may be closed by this pull request
Import issues error instead of warning #808
Closed
jan-petr
jan-petr approved these changes Sep 1, 2021
View reviewed changes
Copy link
Contributor

@jan-petr jan-petr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK

@MichaelStritt MichaelStritt added the bug Something isn't working label Sep 1, 2021
@MichaelStritt
Copy link
Contributor

MichaelStritt commented Sep 1, 2021

Played around with the code a tiny bit. I tried to make it similar to the one from ImportMaster:

...
================================== NIFTI to BIDS CONVERSION ==================================
================================== dataset_description.json ==================================
Missing recommended fields:           HEDVersion, DatasetType, License


==============================================================================================
NII2BIDS failed for anatomical image of Sub103_run-1
Message: This is a test error...
xASL_imp_NII2BIDS_RunAnat, line 26...
Continuing...
================================== BIDS JSON Check Results ===================================
Missing recommended ASL fields:       SliceEncodingDirection, LabelingOrientation
...

It is also included in the x.logging field now, just like in the ImportMaster:

>> x.logging
ans = 
  struct with fields:
    message: 'This is a test error...'
      stack: [9×1 struct]

>> x.logging.stack(1)
ans = 
  struct with fields:
    file: '...\m.stritt\Server_xASL\ExploreASL\Modules\SubModule_Import\xASL_imp_NII2BIDS_RunAnat.m'
    name: 'xASL_imp_NII2BIDS_RunAnat'
    line: 26

>> x.logging.stack(2)
ans = 
  struct with fields:
    file: '...\m.stritt\Server_xASL\ExploreASL\Modules\SubModule_Import\xASL_imp_NII2BIDS_Run.m'
    name: 'xASL_imp_NII2BIDS_Run'
    line: 35

MichaelStritt
MichaelStritt approved these changes Sep 1, 2021
View reviewed changes
Copy link
Contributor

@MichaelStritt MichaelStritt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay for me, maybe @jan-petr needs to recheck my edits though 👍

HenkMutsaerts and others added 4 commits September 1, 2021 16:35
@HenkMutsaerts @MichaelStritt
#808 xASL_imp_NII2BIDS_Run: catch errors
4e7c095
@MichaelStritt
#808 xASL_imp_NII2BIDS_Run: Change to camelcase
0b944a9 
I think for the part after Run we should use camelcase as well. Otherwise this seems like a good addition. I am checking out if we can add this to the logging struct in some kind of way.
@MichaelStritt
#808 xASL_imp_NII2BIDS_...: Use logging feature
f470f7d
@MichaelStritt
#808 Fix headers
003a9b3
@MichaelStritt MichaelStritt force-pushed the bug-#808_CatchErrorsInImport branch from 61ab342 to 003a9b3 Compare September 1, 2021 14:35
@MichaelStritt MichaelStritt merged commit 003a9b3 into release-1.8.0 Sep 1, 2021
@MichaelStritt MichaelStritt deleted the bug-#808_CatchErrorsInImport branch September 1, 2021 14:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jan-petr jan-petr jan-petr approved these changes

+1 more reviewer

@MichaelStritt MichaelStritt MichaelStritt approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@jan-petr jan-petr

Labels

bug Something isn't working

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Import issues error instead of warning

4 participants

@HenkMutsaerts @MichaelStritt @jan-petr