Replace column fixes #1048
Replace column fixes #1048
Conversation
wm75
commented
Dec 10, 2020
wm75
commented
- The tool would previously behave unexpectedly with empty columns at the end of lines (drop columns, retain lines that should be skipped, etc)
- The input datatype is now preserved on output
- The code has also been cleaned a bit
Stripping all whitespace from the end of input lines is questionable in general, and outright wrong in the case of tab-delimited data with empty columns at the end of lines.
| if len(out) == len(line_content): | ||
| output.write('%s\n' % delimiter_local.join(out)) | ||
| if column >= len(line_content): | ||
| continue |
There was a problem hiding this comment.
This is somewhat questionable, but preserves the behavior of the previous version.
There was a problem hiding this comment.
Yes, that's the simple alternative. We could also couple the behavior for short lines to the general mode (skip/error/pass), which would also not be too complicated to do.
I was just reluctant to change this cause it may force people to revise WFs when they update to the new tool version.
But if you think it's worth it I can change it to either of the two behaviors above.
There was a problem hiding this comment.
if we bump the version I think this is fine and afaik the supplied value, if this fails, is invalid
There was a problem hiding this comment.
Ok, I'll change this. Regarding to what:
I can see use-cases for the more complex implementation where users would want to skip/keep unmodified truncated lines instead of erroring out (like certain lines of the input lacking an annotation so there's nothing to replace, but you don't want to lose these lines either) so I'd prefer being able to handle such situations.
|
I'm not sure why, but in containers, those tests all fail. |
|
Yes, but it seems that job execution as such is broken. |
|
Rerunning tests now. |
