Added long read walkers from long-running branch. #8799
Conversation
Fixed warnings in long read tools that were pulled in.
|
Github actions tests reported job failures from actions build 8883008459
|
There was a problem hiding this comment.
A bunch of issues. I mostly didn't touch the actual logic of these tools but rather looked at their documentation and conformity to the GATK style. In general you should update the docs here for each of these tools as there were a bunch of copy-paste errors. Furthermore most of your String outputs can become GATKPath inputs for maximum robustness.
The elephant in the room is that none of this has tests. Thats a broader discussion but I don't want to hold up these experimental tools by asking for lots and lots of test code for it especially when a bunch of them are pretty simple (if not necessarily unambiguously named in all cases).
| * </ul> | ||
| * | ||
| * <h3>Usage Example</h3> | ||
| * <h4>Quickly count errors</h4> |
There was a problem hiding this comment.
wrong docs. It looks like it picked up elements from multiple tools in here
| @Override | ||
| public void apply(GATKRead read, ReferenceContext referenceContext, FeatureContext featureContext ) { | ||
| byte[] quals = new byte[read.getBases().length]; | ||
| Arrays.fill(quals, (byte) 40); |
There was a problem hiding this comment.
should the specific base quality not be configurable?
| * gatk ShardLongReads \ | ||
| * -I input.bam \ | ||
| * -O outputDirectory \ | ||
| * --num-reads-per-split 10000 |
There was a problem hiding this comment.
Include a comment about what the output directory strucutre should look like
| @DocumentedFeature | ||
| @ExperimentalFeature | ||
| @CommandLineProgramProperties( | ||
| summary = "Quickly replace read quals with fixed value (@).", |
There was a problem hiding this comment.
also the wrong documentation here
| oneLineSummary = "Quickly replace read quals with fixed value (@).", | ||
| programGroup = ReadDataManipulationProgramGroup.class | ||
| ) | ||
| public final class ReplaceQuals extends ReadWalker { |
There was a problem hiding this comment.
maybe name it something like OverwriteReadBaseQualities to be a little more explicit.
| doc="Path to which per-read information should be written") | ||
| public String perReadPathName; | ||
|
|
||
| @Argument(fullName = "outputIntervalInfo", |
There was a problem hiding this comment.
required output that is not documented.
| @ExperimentalFeature | ||
| @CommandLineProgramProperties( | ||
| summary = "Count indels", | ||
| oneLineSummary = "Count indels", |
There was a problem hiding this comment.
count indels by interval
| } | ||
|
|
||
| @Override | ||
| public void apply(SimpleInterval interval, ReadsContext readsContext, ReferenceContext referenceContext, FeatureContext featureContext) { |
There was a problem hiding this comment.
this reads context has the potential to be absurdly huge and crash if the intervals are too big. This should probably be warned about somewhere.
| @CommandLineProgramProperties( | ||
| summary = "Count indels", | ||
| oneLineSummary = "Count indels", | ||
| programGroup = LongReadProgramGroup.class |
There was a problem hiding this comment.
This is one of the tools in here that probably can belong into one of the other Program groups if you included much more detailed documentation about what it does.
| oneLineSummary = "Count indels", | ||
| programGroup = LongReadProgramGroup.class | ||
| ) | ||
| public final class CountIndels extends IntervalWalker { |
There was a problem hiding this comment.
probably countIndelsPerInterval
Fixed warnings in long read tools that were pulled in.