-
-
Notifications
You must be signed in to change notification settings - Fork 718
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
Make p2p shuffle submodules private #7186
Conversation
cc @wence- |
400cc5a
to
f8ed836
Compare
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.
Nice -- I haven't taken a detailed look, but in general I like the idea of having private modules instead of a lot of private functions (where it makes sense)
Unit Test ResultsSee test report for an extended history of previous test failures. This is useful for diagnosing flaky tests. 15 files + 5 15 suites +5 6h 23m 24s ⏱️ + 2h 41m 40s For more details on these failures, see this check. Results for commit ed9e843. ± Comparison against base commit 3567d1f. ♻️ This comment has been updated with latest results. |
f8ed836
to
4b9dcba
Compare
4b9dcba
to
5ce0f35
Compare
t = pa.Table.from_pandas(df) | ||
# FIXME: If we do not preserve the index something is corrupting the | ||
# bytestream such that it cannot be deserialized anymore | ||
t = pa.Table.from_pandas(df, preserve_index=True) |
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.
This is the default. Now it's just explicit
5ce0f35
to
d213959
Compare
test_bad_disk is already a known problem. Currently working on it but won't be fixing inthis PR. Other failures are the websocket failures that happen everywhere right now |
Builds on #7185
This makes all shuffle p2p modules "private" and exposes only the extensions (They are imported by the scheduler) and the actual graph generator.
With #7180 this should also include the layer