Skip to content

Make p2p shuffle submodules private#7186

Merged
fjetter merged 5 commits intodask:mainfrom
fjetter:shuffle_ext_refactor_2
Oct 26, 2022
Merged

Make p2p shuffle submodules private#7186
fjetter merged 5 commits intodask:mainfrom
fjetter:shuffle_ext_refactor_2

Conversation

@fjetter
Copy link
Member

@fjetter fjetter commented Oct 25, 2022

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

@fjetter fjetter changed the title Shuffle ext refactor 2 Make p2p shuffle submodules private Oct 25, 2022
@fjetter
Copy link
Member Author

fjetter commented Oct 25, 2022

cc @wence-

@fjetter fjetter force-pushed the shuffle_ext_refactor_2 branch 3 times, most recently from 400cc5a to f8ed836 Compare October 25, 2022 14:08
Copy link
Member

@jrbourbeau jrbourbeau left a 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)

@github-actions
Copy link
Contributor

github-actions bot commented Oct 25, 2022

Unit Test Results

See 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
  3 157 tests +       1    3 065 ✔️ +     12    84 💤  -   13    8 +2 
23 356 runs  +8 209  22 431 ✔️ +7 857  900 💤 +351  25 +1 

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.

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)
Copy link
Member Author

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

@fjetter fjetter force-pushed the shuffle_ext_refactor_2 branch from 5ce0f35 to d213959 Compare October 26, 2022 14:17
@fjetter
Copy link
Member Author

fjetter commented Oct 26, 2022

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

@fjetter fjetter merged commit 5dccad4 into dask:main Oct 26, 2022
@fjetter fjetter deleted the shuffle_ext_refactor_2 branch November 10, 2022 14:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants