-
Notifications
You must be signed in to change notification settings - Fork 0
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
FEAT-#7254: Support right merge/join #12
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Anatoly Myachev <anatoly.myachev@intel.com>
Clone of the PR modin-project/modin#7226 |
My review is in progress 📖 - I will have feedback for you in a few minutes! |
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe changes to the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant MergeFunction
participant MapFunc
User->>MergeFunction: Call row_axis_merge(left, right)
MergeFunction->>MapFunc: Check if "inner" join and single partition
alt Condition met
MergeFunction->>MergeFunction: Set reverted = True
MergeFunction->>MapFunc: Call map_func(right, left)
else
MergeFunction->>MapFunc: Call map_func(left, right)
end
MapFunc->>MergeFunction: Return merged DataFrame
MergeFunction->>User: Return result
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
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.
I have reviewed your code and did not find any issues!
Please note that I can make mistakes, and you should still encourage your team to review your code as well.
@coderabbitai review |
Actions performedReview triggered.
|
/review |
PR Reviewer Guide 🔍(Review updated until commit fa0ea6f)
|
Persistent review updated to latest commit fa0ea6f |
@coderabbitai full review |
Actions performedFull review triggered. |
PR Code Suggestions ✨
|
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.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- modin/core/storage_formats/pandas/merge.py (3 hunks)
Additional comments not posted (4)
modin/core/storage_formats/pandas/merge.py (4)
132-135
: LGTM: Introduction ofreverted
flag.The introduction of the
reverted
flag is a clear and effective way to handle specific merge conditions.
155-158
: LGTM: Conditional logic inmap_func
.The conditional logic in
map_func
based on thereverted
flag ensures the correct order of DataFrame merging.
181-198
: LGTM: Metadata computation adjustments.The adjustments to the metadata computation correctly account for the
reverted
state, ensuring accurate metadata for the merged DataFrames.
Line range hint
199-222
: LGTM: Consistency of repartitioning logic.The repartitioning logic remains consistent with the new changes and optimizations in the merge operation.
User description
What do these changes do?
Blocked on modin-project/modin#7251Performance: 1.7 sec (the PR) vs 3.1 sec (on main) on Ray (8 cores)
flake8 modin/ asv_bench/benchmarks scripts/doc_checker.py
black --check modin/ asv_bench/benchmarks scripts/doc_checker.py
git commit -s
docs/development/architecture.rst
is up-to-dateDescription by Korbit AI
Note
This feature is in early access. You can enable or disable it in the Korbit Console.
What change is being made?
Add support for right merge/join in the
row_axis_merge
function within the Modin library.Why are these changes being made?
This change addresses the need for right merge/join functionality, which was previously unsupported. The implementation ensures that the merge operation correctly handles the right join by swapping the left and right dataframes when necessary and adjusting the metadata computation accordingly. This approach maintains consistency and correctness in the merge results.
PR Type
enhancement
Description
reverted
flag to manage operand swapping during the merge operation.Changes walkthrough 📝
merge.py
Add support for right merge/join with operand swapping
modin/core/storage_formats/pandas/merge.py
reverted
flag to track operand swapping.Summary by CodeRabbit
New Features
Bug Fixes