Skip to content

Conversation

@jan-petr
Copy link
Contributor

@jan-petr jan-petr commented Sep 5, 2022

Linked issue

Closes #1167

@jan-petr jan-petr self-assigned this Sep 5, 2022
@jan-petr jan-petr linked an issue Sep 5, 2022 that may be closed by this pull request
7 tasks
Copy link
Member

@HenkMutsaerts HenkMutsaerts left a comment

Choose a reason for hiding this comment

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

See comments. Also, where did my edits go? (they were about being flexible with the NIfTI naming, using regular expressions, when doing NII2BIDS)

@jan-petr
Copy link
Contributor Author

jan-petr commented Sep 7, 2022

Last thing - regarding adding similar warnings as you did in the alternative commit: There is no need to add those because they are already there. You didn't add a new warning, you have simply taken an existing one and moved it to a different location. So that warning is still there in xASL_imp_NII2BIDS_RunPerf.m at line 58.

Copy link
Contributor

@BeatrizPadrela BeatrizPadrela left a comment

Choose a reason for hiding this comment

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

Works nicely

@HenkMutsaerts
Copy link
Member

Last thing - regarding adding similar warnings as you did in the alternative commit: There is no need to add those because they are already there. You didn't add a new warning, you have simply taken an existing one and moved it to a different location. So that warning is still there in xASL_imp_NII2BIDS_RunPerf.m at line 58.

That's not what I meant. In the code that Bea and me changed, you want a warning if an incorrect filename was detected (which you said is impossible, but it will still help us). And to improve the warnings there, to be a bit more explanatory.

@jan-petr
Copy link
Contributor Author

jan-petr commented Sep 8, 2022

Last thing - regarding adding similar warnings as you did in the alternative commit: There is no need to add those because they are already there. You didn't add a new warning, you have simply taken an existing one and moved it to a different location. So that warning is still there in xASL_imp_NII2BIDS_RunPerf.m at line 58.

That's not what I meant. In the code that Bea and me changed, you want a warning if an incorrect filename was detected (which you said is impossible, but it will still help us). And to improve the warnings there, to be a bit more explanatory.

You mean, not a warning that the file was missing, but a warning that a similar filename was detected, but not according to what was expected?

@jan-petr
Copy link
Contributor Author

jan-petr commented Sep 9, 2022

Now the warnings are similar to as they were in your commits.

Copy link
Member

@HenkMutsaerts HenkMutsaerts left a comment

Choose a reason for hiding this comment

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

Nice. Minor cosmetics, we should always add a space ' ' after a comma in Matlab (I learned from somebody :) and you put ( ) that are not required in Matlab.

@jan-petr jan-petr merged commit 6f14c41 into develop Sep 12, 2022
@jan-petr jan-petr deleted the bug-#1167_nii2BIDS_new branch September 12, 2022 15:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Oslo data - HAD8 import issue with creating ASL/M0 jsons

4 participants