Fix bai location when output not specified for BuildBamIndex #1838
Conversation
There was a problem hiding this comment.
@kachulis I have a branch with the fix I proposed below, and a test, but I haven't yet fixed the test issue I mentioned below with the existing tests (writing into the testdata directory). I'll finish that and get it submitted.
| @@ -88,7 +88,7 @@ protected int doWork() { | |||
| // set default output file - input-file.bai | |||
| if (OUTPUT == null) { | |||
|
|
|||
| final String baseFileName = inputPath.getFileName().toString(); | |||
| final String baseFileName = INPUT.hasFileSystemProvider() ? inputPath.toAbsolutePath().toString() : inputPath.getFileName().toString(); | |||
There was a problem hiding this comment.
The best fix for this would be to change OUTPUT to have the same type (PicardHtsPath) as INPUT, so that they have the same semantics. Keeping OUTPUT as File while INPUT is a PicardHtsPath will most likely cause problems with cloud inputs.
There was a problem hiding this comment.
Awesome, I was hoping you might have a better solution to this :)
| /* First sort, before indexing */ | ||
| new SortSam().instanceMain(new String[]{ | ||
| "I=" + INPUT_FILE, | ||
| "O=" + OUTPUT_SORTED_FILE, |
There was a problem hiding this comment.
The test is called "NoOutputSpecified", but it specifies an output .
There was a problem hiding this comment.
The existing tests in this file are already a bit sketchy, since they write their outputs into the testdata directory. Its not clear if they're actually overwriting test data or not (I don't think they are, but it LOOKS like they are).
|
@kachulis What happened to this branch? Was it abandoned on purpose or by accident? |
|
Closed on purpose because @cmnbroad said he had a better solution. Not sure if that ever got implemented. |
|
We're closing this and adding it to the list of things to be cloudified. |
Addresses #1827