Remove PileupFilter and revamp InputFilters #605
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Merged
- LociParser → ParsedLoci, removed state+mutation pattern, leaving an
object that just wraps loci, after we've parsed strings, but before
we have contig-lengths / a LociSet
- uses ParsedLociRange and ParsedLociRanges classes, which
encapsulate "all loci" special-casing.
- parseLoci method is moved in there, since it's basically a
constructor for a ParsedLoci.
- removed all non-test calls of unsafe ParsedLoci.result and
LociSet(String), that assume no contig-lengths are needed because
provided ranges will be closed.
- changed some callers to reflect that the ReadSets() call now
integrates building loci and filters in addition to loading
read-sets.
- move a couple of InputFilters ctors that were only used in tests to
TestInputFilters
|
Looks great, thanks for cleaning all this up. While reviewing I was inspired to do a further loci-parsing refactor that I'd attempted and failed a few times in the past, but finally cracked this time: #606. A couple small things in #606 are separable and more like bona-fide tweaks to what is here, but on the whole it's a separate PR and can be reviewed/merged as such. I opened #606 against this in case you want to merge it in here, but if not you can just merge this and I'll send out #606 as a fresh PR. |
ryan-williams
approved these changes
Oct 6, 2016
loci-parsing refactor
ryan-williams
added a commit
to ryan-williams/guacamole
that referenced
this pull request
Oct 6, 2016
ryan-williams
added a commit
to ryan-williams/guacamole
that referenced
this pull request
Oct 6, 2016
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR:
PileupFilterfiles and usage fromSomaticStandardReadFilterArgswhich has flags to set properties ofInputFiltersLociArgsandReadFilterArgstied to read inputs hierarchy as opposed to partitioning.InputFiltersto tie in with arg-parsing and removing some defaultsLociParserresult inNonewhen no loci are specified and removing thefallbackarguments as sometimes this strings were incorrectly specified.InputFiltersin a few cases. In 3bcca66SomaticStandardwent from removing duplicate and failed reads to including them because of a default filter argument, so I wanted to avoid that.