fix(rust,python): Fix possibly incorrect order of columns when using ipc stream with_columns
#14859
Conversation
mickvangelderen
requested review from
ritchie46,
stinodego,
orlp and
c-peters
as code owners
github-actions
bot
added
internal
stinodego
dismissed
their stale review
Actually, looks like this now results in a test failure - seems like we'll have to fix the bug or skip this test for now and fix it later. The addition of assert is good regardless.
mickvangelderen
requested review from
alexander-beedie and
MarcoGorelli
as code owners
mickvangelderen
force-pushed
the
fix-missing-test-assertions-in-ipc-stream
branch
from
f6d7bac to
60af176
Compare
mickvangelderen
changed the title
with_columns
|
@stinodego the failing tests indicated an actual problem. I've addressed this now. The commits are pretty decent so this can be reviewed on a per-commit basis. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #14859 +/- ##
==========================================
+ Coverage 80.98% 81.01% +0.02%
==========================================
Files 1331 1331
Lines 172525 172752 +227
Branches 2455 2455
==========================================
+ Hits 139720 139955 +235
+ Misses 32337 32329 -8
Partials 468 468 ☔ View full report in Codecov by Sentry. |
|
@ritchie46 I would like to move this forward and would like to get your input on the following:
|
| fn assert_df_eq_passes() { | ||
| let df = df!("a" => [1], "b" => [2]).unwrap(); | ||
| assert_df_eq!(df, df); | ||
| drop(df); // Ensure `assert_df_eq!` does not consume its arguments. |
There was a problem hiding this comment.
Does assert_eq take a ref by default?
There was a problem hiding this comment.
I think so, although it is not entirely obvious to me why the implementation of assert_eq is what it is.
No, it's a fix. |
No description provided.