Fixes #746 Record ExploreASL version on Import by MichaelStritt · Pull Request #773 · ExploreASL/ExploreASL

MichaelStritt · 2021-08-16T09:48:50Z

Linked issue

jan-petr

This is good (I made minor changes only - all committed already). But not all I wanted - see the issue description. I wrote some explanations as well so that you get the big picture of why we need that ;) Cause we will need to implement a lot in 1.9.0 which is related to these versions.

MichaelStritt · 2021-08-16T18:41:58Z

Note to myself: also fix the "RandomText" things here 👍

Edit: don't really think this is a problem anymore. If I remove those fields from the original studyPar, they don't get added as that.

...
                Name: '027_S_5079'
         BIDSVersion: '1.6.0'
         DatasetType: 'raw'
    Acknowledgements: 'Imported with ExploreASL 1.8.0_BETA'
    HowToAcknowledge: 'Please cite this paper: https://www.ncbi.nlm.nih.gov/pubmed/001012092119281'
...

MichaelStritt · 2021-08-16T20:45:58Z

You can run UnitTest = xASL_ut_function_xASL_bids_CheckDatasetDescription(TestRepository) to test the xASL_bids_CheckDatasetDescription script.

MichaelStritt · 2021-08-16T21:04:40Z

@jan-petr: We're going to have two minor merge conflicts with #778, but otherwise it should be fine. I even added a unit test. Already looking forward to the feedback. Hope we can get the ADNI things fixed this week, so I can reprocess some data 😄

jan-petr · 2021-08-17T08:17:28Z

Note to myself: also fix the "RandomText" things here

Edit: don't really think this is a problem anymore. If I remove those fields from the original studyPar, they don't get added as that.
...
                Name: '027_S_5079'
         BIDSVersion: '1.6.0'
         DatasetType: 'raw'
    Acknowledgements: 'Imported with ExploreASL 1.8.0_BETA'
    HowToAcknowledge: 'Please cite this paper: https://www.ncbi.nlm.nih.gov/pubmed/001012092119281'
...

Yes of course you are right. RandomText has been removed long time ago - it is only in studyPar.jsons and we can keep it there, no need to remove it from there and from reference in all cases.

jan-petr

All looks very good. Including the test! One thing we need to improve. The "Imported with ExploreASL" is potentially not the only text in the Acknowledgement - we need to be able to deal with extra text - see comments inside the code.

Functions/xASL_bids_CheckDatasetDescription.m

Functions/xASL_bids_CreateDatasetDescriptionTemplate.m

Functions/xASL_bids_CheckDatasetDescription.m

Testing/UnitTests/xASL_ut_function_xASL_bids_CheckDatasetDescription.m

jan-petr

Thanks for the fixes - it looks good. Two more comments, that weren't somehow submitted before...

First - we don't create the whole acknowledgement - we load it from studyPar and add our ExploreASL part at the end.
Second - now that we export the version, it will mess up the Flavors - i.e. the reference and new import will always differ because everything was done with the older version - shall we ignore the acknowledgement field in the comparison script?!

Functions/xASL_bids_CreateDatasetDescriptionTemplate.m

Functions/xASL_bids_CheckDatasetDescription.m

MichaelStritt · 2021-08-17T22:09:50Z

Example before changes `9712786`

>> [identical,results] = xASL_bids_CompareStructures(pathA,pathB,1,[],1,0);
====================================================================================================
Dataset:   x027_S_5079
           No missing files
====================================================================================================
Dataset:   x027_S_5079_b
Missing:   sourceStructure.json
====================================================================================================
Identical: false
RMSE:      2.13
Min.diff.: 118.00
Max.diff.: 1428.00
File:      ASL4D.nii
           RMSE of NIFTIs above threshold.
Text file: Neues Textdokument.txt
           Different file content.
File:      \rawdata\dataset_description.json
           Different value: Acknowledgements (Imported with ExploreASL 1.8.0_BETA. vs Imported with ExploreASL 1.9.0_BETA.)

Example after changes `9712786`

>> [identical,results] = xASL_bids_CompareStructures(pathA,pathB,1,[],1,0);
====================================================================================================
Dataset:   x027_S_5079
           No missing files
====================================================================================================
Dataset:   x027_S_5079_b
Missing:   sourceStructure.json
====================================================================================================
Identical: false
RMSE:      2.13
Min.diff.: 118.00
Max.diff.: 1428.00
File:      ASL4D.nii
           RMSE of NIFTIs above threshold.
Text file: Neues Textdokument.txt
           Different file content.

jan-petr

OK

Added described functionality. Added unit test. Added BIDS2Legacy user feedback.

MichaelStritt requested a review from jan-petr August 16, 2021 09:48

MichaelStritt linked an issue Aug 16, 2021 that may be closed by this pull request

Record ExploreASL version on Import #746

Closed

MichaelStritt self-assigned this Aug 16, 2021

MichaelStritt added the optimization Ensure that code runs faster with unchanged functionality label Aug 16, 2021

jan-petr force-pushed the develop branch from c5f9e60 to 1b3a642 Compare August 16, 2021 11:13

jan-petr requested changes Aug 16, 2021

View reviewed changes

MichaelStritt requested a review from jan-petr August 16, 2021 21:05

MichaelStritt changed the title ~~Fixes #746 acknowledge version~~ Fixes #746 Record ExploreASL version on Import Aug 17, 2021

jan-petr requested changes Aug 17, 2021

View reviewed changes

MichaelStritt requested a review from jan-petr August 17, 2021 13:46

jan-petr requested changes Aug 17, 2021

View reviewed changes

Functions/xASL_bids_CreateDatasetDescriptionTemplate.m Show resolved Hide resolved

Functions/xASL_bids_CheckDatasetDescription.m Show resolved Hide resolved

MichaelStritt requested a review from jan-petr August 17, 2021 22:10

jan-petr assigned jan-petr and unassigned MichaelStritt Aug 18, 2021

jan-petr approved these changes Aug 18, 2021

View reviewed changes

MichaelStritt and others added 10 commits August 18, 2021 14:50

#746 xASL_imp_NII2BIDS: Add version to dataset_description.json

d0c53a1

#746 xASL_imp_NII2BIDS: Add version to dataset_description.json (2)

9722a32

#746 Minor function admin changes

b4c8be8

#746 xASL_bids_CheckDatasetDescription

b6d8c5a

Added described functionality. Added unit test. Added BIDS2Legacy user feedback.

#746 Import version check: Edit warning messages

63fc2c3

#746 xASL_bids_...DatasetDescription: Improve code functionality

ea515ff

#746 xASL_bids_CreateDatasetDescriptionTemplate: handle existing field

20aa21f

#746 xASL_bids_CreateDatasetDescriptionTemplate: minor fix

d597afb

#746 xASL_bids_Compare...: Cosmetics

f5dfff2

#746 xASL_bids_Compare...: add ignore functionality

a98cd8e

#746 xASL_bids_CompareStructures: Cosmetics

9c44d40

jan-petr force-pushed the optimize-#746_AcknowledgeVersion branch from e4b71cf to 9c44d40 Compare August 18, 2021 13:06

jan-petr merged commit 9c44d40 into develop Aug 18, 2021

jan-petr deleted the optimize-#746_AcknowledgeVersion branch August 18, 2021 13:07

MichaelStritt mentioned this pull request Aug 20, 2021

Consistency check of Acknowledgment and GeneratedBy field in dataset_description.json #793

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixes #746 Record ExploreASL version on Import#773

Fixes #746 Record ExploreASL version on Import#773
jan-petr merged 11 commits intodevelopfrom
optimize-#746_AcknowledgeVersion

MichaelStritt commented Aug 16, 2021

Uh oh!

jan-petr left a comment

Uh oh!

MichaelStritt commented Aug 16, 2021 •

edited

Loading

Uh oh!

MichaelStritt commented Aug 16, 2021

Uh oh!

MichaelStritt commented Aug 16, 2021

Uh oh!

jan-petr commented Aug 17, 2021

Uh oh!

jan-petr left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jan-petr left a comment

Uh oh!

Uh oh!

Uh oh!

MichaelStritt commented Aug 17, 2021

Uh oh!

jan-petr left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

MichaelStritt commented Aug 16, 2021

Linked issue

Uh oh!

jan-petr left a comment

Choose a reason for hiding this comment

Uh oh!

MichaelStritt commented Aug 16, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MichaelStritt commented Aug 16, 2021

Uh oh!

MichaelStritt commented Aug 16, 2021

Uh oh!

jan-petr commented Aug 17, 2021

Uh oh!

jan-petr left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jan-petr left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

MichaelStritt commented Aug 17, 2021

Example before changes 9712786

Example after changes 9712786

Uh oh!

jan-petr left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

MichaelStritt commented Aug 16, 2021 •

edited

Loading

Example before changes `9712786`

Example after changes `9712786`