Skip to content

[PERF] Avoid materializing not-equal pairs for keep first or last - #1681

Open
tunglambk wants to merge 4 commits into
pyjanitor-devs:devfrom
tunglambk:perf/not-equal-keep-selection
Open

[PERF] Avoid materializing not-equal pairs for keep first or last#1681
tunglambk wants to merge 4 commits into
pyjanitor-devs:devfrom
tunglambk:perf/not-equal-keep-selection

Conversation

@tunglambk

@tunglambk tunglambk commented Aug 24, 2026

Copy link
Copy Markdown

PR Description

  • select the first or last unequal right row from at most two candidates instead of materializing every unequal pair
  • preserve the existing materialized-path ordering and null semantics
  • cover duplicates, sorted and unsorted inputs, single-value inputs, nullable integers, datetimes, floating NaN, all-null data, and empty inputs

ELI5

For any left value, the first different right value is either the first right value or, if those are equal, the first value different from it. The same rule works backwards for keep="last", so the join only needs two candidates rather than a nearly Cartesian list of pairs.

Performance

End-to-end timings for 2,000 left and 2,000 right rows (best of three runs):

Cardinality Keep Before After Speedup
2 first 47.252 ms 3.541 ms 13.3x
2 last 50.935 ms 3.529 ms 14.4x
2,000 first 87.863 ms 3.451 ms 25.5x
2,000 last 83.081 ms 3.441 ms 24.1x

Correctness

  • a direct-scan oracle covers the edge cases listed above for both keep modes
  • an explicit regression test preserves the legacy output order
  • 2,000 randomized int64, nullable-integer, and floating-NaN trials matched the materializing implementation
  • keep="all" remains on the existing code path

Validation

  • focused conditional-join suite: 259 passed, 1 skipped
  • full suite: 1,358 passed, 5 skipped, 58 xfailed, 7 xpassed; the single sandbox DNS failure passed when rerun with network access
  • all relevant pre-commit hooks passed on the changed Python files

This PR resolves #1651.

PR Checklist

  1. PR from a feature branch on a fork.
  2. Add a line to CHANGELOG.md under the latest version header.

Relevant Reviewers

@codecov

codecov Bot commented Aug 24, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 85.86%. Comparing base (901f4b3) to head (d64e55e).
⚠️ Report is 218 commits behind head on dev.

Additional details and impacted files
@@            Coverage Diff             @@
##              dev    #1681      +/-   ##
==========================================
- Coverage   87.56%   85.86%   -1.70%     
==========================================
  Files          95      125      +30     
  Lines        6819    10018    +3199     
==========================================
+ Hits         5971     8602    +2631     
- Misses        848     1416     +568     
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@tunglambk
tunglambk force-pushed the perf/not-equal-keep-selection branch from 14749eb to 287fcf4 Compare August 24, 2026 08:09
Comment thread CHANGELOG.md Outdated

@samukweku samukweku left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks @tunglambk

kindly update the docs as well, informing users about this approach for != join

@tunglambk
tunglambk force-pushed the perf/not-equal-keep-selection branch from 287fcf4 to d64e55e Compare August 26, 2026 07:08
@tunglambk

tunglambk commented Aug 26, 2026

Copy link
Copy Markdown
Author

Addressed the review in d64e55e: the changelog now credits @tunglambk, and the conditional_join documentation describes the optimized single-!= path for keep="first" and keep="last". I also rebased onto the current dev. Locally, the full conditional_join test file passes (296 passed, 1 skipped), and the documentation build completes successfully. The CI documentation job reaches the preview deployment step, then receives a 403 when github-actions[bot] pushes to the upstream gh-pages branch.

@samukweku

Copy link
Copy Markdown
Collaborator

Thanks @tunglambk

@samukweku

Copy link
Copy Markdown
Collaborator

@tunglambk kindly merge

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.

[PERF] Avoid materializing all != matches for keep first/last

2 participants