Ensuring transform and log files are generated #62
Conversation
Signed-off-by: neuronflow <[email protected]>
Signed-off-by: neuronflow <[email protected]>
|
🤖 Code Formatting Reminder Hello there! 👋 It looks like the code in this pull request might benefit from some formatting improvements. |
|
@sarthakpati I introduced a separate test, testing the transform function now. |
|
/format |
|
🤖 I will now format your code with black. Check the status here. |
Signed-off-by: neuronflow <[email protected]>
|
🤖 Code Formatting Reminder Hello there! 👋 It looks like the code in this pull request might benefit from some formatting improvements. |
Signed-off-by: neuronflow <[email protected]>
|
/format |
|
@sarthakpati okay...the single tests pass but the combined test fails |
|
🤖 I will now format your code with black. Check the status here. |
Signed-off-by: neuronflow <[email protected]>
|
🤖 Code Formatting Reminder Hello there! 👋 It looks like the code in this pull request might benefit from some formatting improvements. |
data/tcia_aaac_t1ce_transform.mat
Outdated
There was a problem hiding this comment.
should this file be part of the repo?
There was a problem hiding this comment.
I believe we need a mat file for the transformation test. Is this the file that is used there?
There was a problem hiding this comment.
ah yeah probably, checks out
ereg/registration.py
Outdated
| log_file (str): The log file. | ||
| logger_type (str): The logger type. | ||
| """ | ||
| reload(logging) |
There was a problem hiding this comment.
There was a problem hiding this comment.
that also puzzled me :)
There was a problem hiding this comment.
I don't have a lot of experience with the logging library (I rarely use it in my usual code, since all console messages either get saved by the HPC or get pushed to files). Anyway, I am happy to use whatever solution you propose that works for @neuronflow.
|
|
|
Weird. 91fb002 should check for any empty log files and throw an exception. |
|
Rechecked and it works fine for me. Perhaps you had an older version installed that is being sued instead of the local repo that includes the changes? => Resolved in call, suer error (logs empty because target files already existed, will add a warning for that) |
fix: 3 log file issue cleanup
cleanup and fixes
|
rewrote the usage of the logging module and tests. |
|
I am fine with the changes. However, who should review/merge the PR now that @MarcelRosier you have contributed to the code? |
|
just checked, seems to work fine. Thanks @MarcelRosier :) ..Only small issue is if the trans files already exist ereg does not really warn..this might lead to unexpected behavior..we might open a feature request for this. I will merge this now as both @sarthakpati and me approve the changes. |
this bugfix PR also introduces a functional wrapper for resampling