chore: Remove unused and confusing match arm #23691
Conversation
github-actions
bot
added
internal
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #23691 +/- ##
==========================================
+ Coverage 80.84% 81.32% +0.48%
==========================================
Files 1642 1643 +1
Lines 223086 223135 +49
Branches 2837 2837
==========================================
+ Hits 180352 181472 +1120
+ Misses 42035 40964 -1071
Partials 699 699 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| AggState::AggregatedList(column) | ||
| }, | ||
| (true, _) => { | ||
| let series = match aggregated { |
There was a problem hiding this comment.
I'll trust you on the details that we don't have to make an AggregatedList here but...
match boolean {
true => a,
_ => b,
}This is just if/else and I have no idea how clippy isn't screaming at you 😅
There was a problem hiding this comment.
hahaha, yes true
| @@ -204,16 +204,11 @@ impl<'a> AggregationContext<'a> { | |||
| groups: Cow<'a, GroupPositions>, | |||
| aggregated: bool, | |||
| ) -> AggregationContext<'a> { | |||
| let series = match (aggregated, column.dtype()) { | |||
There was a problem hiding this comment.
This was only called at 2 places.
- One had
aggregated=false - The other was
count.rs, hadaggregated=true, but had an constant datatype ofIDX_DTYPE.
There was a problem hiding this comment.
But could it be hit with an aggregate=true and a List dtype?
There was a problem hiding this comment.
No, no such path could be hit. The above two paths were the only ones.
No description provided.