-
Notifications
You must be signed in to change notification settings - Fork 130
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
filter: Allow --group-by week
, refactoring
#1067
Conversation
Group by day is not practical because: 1. The amount of groups would overly bias temporal diversity 2. In many datasets, daily sequencing volume varies between days of the week.
Put the generated columns in a set so it can be added to more easily.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1067 +/- ##
==========================================
+ Coverage 61.68% 61.77% +0.09%
==========================================
Files 52 52
Lines 6300 6316 +16
Branches 1585 1550 -35
==========================================
+ Hits 3886 3902 +16
Misses 2141 2141
Partials 273 273 ☔ View full report in Codecov by Sentry. |
89df17d
to
c4df460
Compare
6c21ae2
to
a6ec84a
Compare
a6ec84a
to
c0abbda
Compare
c0abbda
to
f2a6d71
Compare
This PR now has some downstream PRs. Ideally, I'd like to merge in this order to avoid merge conflicts:
but I can rebase if needed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just one comment, but looks good to merge 👍
I'll move on to the other PRs 😄
f2a6d71
to
3043718
Compare
Description of proposed changes
Some group-by cleanup and implementation of
--group-by week
.Related issue(s)
--group-by month
#960.Testing
Checklist