-
Notifications
You must be signed in to change notification settings - Fork 13
Closes #940 Check folder hierarchy #955
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.
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
Conversation
jan-petr
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Besides XML, I have one issue:
- What if the Extension is given without the fullstop - just "PAR", but then just "PAR" can be mistaken with something else. But we can't look at it as the last string "PAR$" cause there can be some bracket etc.
I have no real idea how to solve this. Better to send to Henk for review as well - even though it will take some time...
jan-petr
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm fine with it and let's see if Henk has any idea how to address my comment in PR...
HenkMutsaerts
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should add an elseif for another (unknown) extension, then it can just issue a warning that we detected an unknown extension.
MINOR:
I would replace the regular expressions by a single one, then you can do the regexp once outside the if statement.
hasKnownExtension = ~isempty(regexpi(lastElement,'.(dcm|ima|xml|par|rec)'));
and you can combine else and if on line 84 into elseif.
Otherwise nice!
Try using Matlab's |
jan-petr
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small edit needed
jan-petr
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK
c503604 to
a241e0a
Compare
Linked issue
Check out #940