-
Notifications
You must be signed in to change notification settings - Fork 128
ENH: Allow InitialTransformParameterFileName relative to parameter file #907
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
ENH: Allow InitialTransformParameterFileName relative to parameter file #907
Conversation
|
|
||
| /** We can safely read the initial transform. */ | ||
| this->ReadInitialTransformFromFile(fileName.c_str()); | ||
| const auto lastConfigurationParameterFilePathSeparator = configurationParameterFileName.find_last_of("\\/"); |
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.
does itksys not have a robust function for what you are trying to do here?
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.
does itksys not have a robust function for what you are trying to do here?
Good question 😃 I found some itksys functions that do similar things, by splitting a path to its components, and then joining them again:
static std::vector<std::string> SplitString(const std::string& s,
char separator = '/',
bool isPath = false);
static std::string Join(const std::vector<std::string>& list,
const std::string& separator);
But as far as I can see, it involves extra manual programming, it doesn't get much easier than what it does now: Just find the last separator (slash or backslash) in the current transform file path (by std::string::find_last_of), and then concatenate the current path up to that last separator with the string specified by "InitialTransformParameterFileName".
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, just at least add a comment explaining what you do here.
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, thanks! I just added some more comment, and also made the code clearer by introducing a local variable, isAbsoluteFilePath. Hope it's clear enough now!
| if (lastConfigurationParameterFilePathSeparator != std::string::npos && !startsWithFilePathSeparator && | ||
| !hasDriveLetter && !itksys::SystemTools::FileExists(fileName)) | ||
| { |
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 could reverse if and else, and then do:
if (lastConfigurationParameterFilePathSeparator == std::string::npos
|| startsWithFilePathSeparator
|| hasDriveLetter
|| itksys::SystemTools::FileExists(fileName))
To make the condition slightly simpler, maybe.
Could even just test on itksys::SystemTools::FileExists(fileName) and forget about the other three conditions. Might make the code slightly slower sometimes, but slightly easier to read.
Allow the file name specified by the "InitialTransformParameterFileName" parameter in a transform parameter file to be relative to that transform parameter file.
0e96c2f to
088ec11
Compare
|
Note that the pull request still supports "InitialTransformParameterFileName" file paths relative to the current working directory as well (rather than relative to the directory of the current transform parameter file). The current working directory is typically where the executable is started. This is important for backward compatibility, but still for the current executables as well. For example, suppose elastix is started in an input data directory, "C:\InputDir": Creates a file "D:\OutputDir\TransformParameters.0.txt" having: While the file InitialTransformParameters.txt could be located in "C:\InputDir". This still works fine after this pull request. |
Allow the file name specified by the "InitialTransformParameterFileName" parameter in a transform parameter file to be relative to that transform parameter file.