[PERF] Simplify the conditional_join Python/Rust boundary - #1672
Open
samukweku wants to merge 2 commits into
Open
[PERF] Simplify the conditional_join Python/Rust boundary#1672samukweku wants to merge 2 commits into
samukweku wants to merge 2 commits into
Conversation
Remove a duplicate _sum_starts_ends definition, replace ~87 repeated per-call dtype-dispatch dicts in _agg_functions.py/_compare.py/ _binary_search.py with one cached dispatcher (_rs_func), and swap repeat_index calls for np.repeat, which benchmarks faster at every scale tested and is more immediately readable. Issue #1649 @samukweku Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01V3KAbkp6JV96KYXNc6EJmN
Contributor
|
samukweku
marked this pull request as draft
August 21, 2026 22:27
samukweku
marked this pull request as ready for review
August 21, 2026 22:32
samukweku
force-pushed
the
1649-simplify-conditional-join-rust-boundary
branch
from
August 26, 2026 04:22
35b31ef to
db93539
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
_sum_starts_endsdefinition in_agg_functions.py(the later copy was silently shadowing the first)._agg_functions.py,_compare.py, and_binary_search.pywith one cached dispatcher,_rs_func(newjanitor/functions/_conditional_join/_dtype_dispatch.py). Unsupported-dtype errors keep the exact sameKeyErrormessage as before.janitor_rs.repeat_indexfornp.repeatat all 6 maintained-path call sites (_helpers.py×4,_range_indices.py,_greater_than_indices.py,_less_than_indices.py).repeat_indexis a literalnp.repeatequivalent (verified injanitor-rs/src/index_builder.rs) and benchmarks faster.reorder_index, positional-index construction) are untouched and stay in Rust.janitor_rs.repeat_indexitself is now unused on the maintained path but is left in place injanitor-rsfor now — flagging here for coordinated removal in a separatepyjanitor-devs/janitor-rsPR, per this issue's proposed scope.Benchmarks
repeat_index(Rust) vsnp.repeat,timeit-measured, mean of several reps:repeat_indexnp.repeatNo regression at any scale tested; consistent with the ~8-12% improvement cited in the issue.
Test plan
pytest tests/functions/test_conditional_join.py— 247 passed, 1 skipped (pre-existing, unrelated)_rs_funcdispatch correctness, unsupported-dtypeKeyErrorparity,np.repeat/repeat_indexoutput parity (empty, zero-counts, mixed-counts, 1000-row)pre-commit run --all-fileson changed files (ruff check, ruff format, pydoclint, interrogate) — all passIssue #1649
Co-Authored-By: Claude Sonnet 5 noreply@anthropic.com
Claude-Session: https://claude.ai/code/session_01V3KAbkp6JV96KYXNc6EJmN