filter: Explicitly drop date/year/month columns from metadata during grouping #967
Conversation
Codecov Report
@@ Coverage Diff @@
## master #967 +/- ##
==========================================
- Coverage 59.32% 59.26% -0.06%
==========================================
Files 50 50
Lines 6259 6265 +6
Branches 1585 1588 +3
==========================================
Hits 3713 3713
- Misses 2285 2290 +5
- Partials 261 262 +1
Continue to review full report at Codecov.
|
58bae45 to
e669568
Compare
There was a problem hiding this comment.
Phew, that was a lot of context to catch up on in Slack and the related issue. It makes sense to me to only support grouping by year/month that is generated from the 'date' column 👍
I left some suggestions on the help and warning messages to make this behavior clearer to the users. Also, should there be a separate test for custom 'month' column since this is technically handled separately than the 'year' column?
fd3fd12 to
1f53b63
Compare
- `date` column should not be used for grouping - use generated columns instead. - Any `year`/`month` columns in original metadata should be overridden by generated columns. Without dropping explicitly, a cryptic pandas ValueError occurs. Co-authored-by: Jover Lee <joverlee521@gmail.com>
Co-authored-by: Jover Lee <joverlee521@gmail.com>
1f53b63 to
0e3f122
Compare
Description of proposed changes
datecolumn should not be used for grouping - use generated columns instead.year/monthcolumns in original metadata should be overridden by generated columns. Without dropping explicitly, a cryptic pandas ValueError occurs.--group-byRelated issue(s)
yearcolumn in metadata, crashes withValueError#871Testing
Added a functional test.