You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Nothing really major, but I noticed that the argument max.downsample in the function CoveragePlot (and its relatives) might be slightly misleading in terms of its role.
In parts of the code where max.downsample and downsample.rate are compared, the higher value between the two is selected:
If max.downsample is higher, the resulting plot shows at least that many data points.
This seems more like a minimum threshold rather than a maximum, which the current name might suggest.
The documentation in visualization.R script also notes: @param max.downsample Minimum number of positions kept when downsampling.
Although it's explained in the documentation, I believe a name like min.downsample, or something that reflects its role as a minimal threshold would make functions more intuitive.
As an end user, I'm not fully aware of the difficulties involved in making this change,
but I wanted to raise it as a suggestion.
Thanks for the wonderful package! Hope this feedback was helpful.
The text was updated successfully, but these errors were encountered:
Nothing really major, but I noticed that the argument
max.downsample
in the functionCoveragePlot
(and its relatives) might be slightly misleading in terms of its role.In parts of the code where
max.downsample
anddownsample.rate
are compared, the higher value between the two is selected:If
max.downsample
is higher, the resulting plot shows at least that many data points.This seems more like a minimum threshold rather than a maximum, which the current name might suggest.
The documentation in
visualization.R
script also notes:@param max.downsample Minimum number of positions kept when downsampling.
Although it's explained in the documentation, I believe a name like
min.downsample
, or something that reflects its role as a minimal threshold would make functions more intuitive.As an end user, I'm not fully aware of the difficulties involved in making this change,
but I wanted to raise it as a suggestion.
Thanks for the wonderful package! Hope this feedback was helpful.
The text was updated successfully, but these errors were encountered: