Closed csoneson closed 2 weeks ago
Some things to discuss:
TAG
also retains (e.g.) NNN
- don't allow this (N
in the query are fine, but remove hits where the provided sequence context contains N
). .removeAllNAPositions()
, do we allow a summary assay (and if so, check for rowSums > 0
instead of presence of any non-NA values?Just my 2-cents to the questions above:
N
-containing sequence contexts: As discussed, N
s in rowData(se)$sequence.context
may result both from N
s in the reference as well as from N
s added to sequence contexts near the boundaries that run out of a reference sequence. I agree that we probably don't want to allow this..filterPositionsBy...
, but I am not sure that would be worth it, as the current names are clear and they are internal functions usually called via filterPositions()
.addReadsSummary
very elegant :-) .removeAllZeroPositions
, and a new argument in filterPositions(..., assay.type.zero)
), if we need it. I have a question regarding the coverage filter, at this line: I guess our assays always have row names, but is it safer/required to go via rownames
, or would a logical keep
also work and be more general?
Thanks!
I have a question regarding the coverage filter, at this line: I guess our assays always have row names, but is it safer/required to go via rownames, or would a logical keep also work and be more general?
You're right, we don't need rownames - the two objects (mat
and se
) should always have the same rows anyway.
remove NA positions: I would maybe rather implement such a function separately under a different name (.removeAllZeroPositions, and a new argument in filterPositions(..., assay.type.zero)), if we need it.
Also makes sense to me
rownames
matching for subsetting in 2dac82emin.nbr.samples
argument to .filterPositionsByCoverage
in 2dac82eIncluded in #22