FEAT-#7254: Support right merge/join #42

korbit-ai · 2024-08-15T16:01:25Z

User description

What do these changes do?

Performance: 1.7 sec (the PR) vs 3.1 sec (on main) on Ray (8 cores)

import modin.pandas as pd
from time import time
import numpy as np
from modin.utils import execute

np.random.seed(42)

df1 = pd.DataFrame(np.random.randint(0, 100_000,size=(400_000, 64)))
df2 = pd.DataFrame(np.random.randint(0, 100_000,size=(400_000, 64)))
execute(df1)
execute(df2)


for _ in range(3):
    start = time()
    result = pd.merge(df1, df2, how="right", on=1)
    execute(result)
    print(f"{result.shape=}")
    print(f"merge time: {time()-start}")

first commit message and PR title follow format outlined here

NOTE: If you edit the PR title to match this format, you need to add another commit (even if it's empty) or amend your last commit for the CI job that checks the PR title to pick up the new PR title.
passes flake8 modin/ asv_bench/benchmarks scripts/doc_checker.py
passes black --check modin/ asv_bench/benchmarks scripts/doc_checker.py
signed commit with git commit -s
Resolves #7254
tests added and passing
module layout described at docs/development/architecture.rst is up-to-date

Description 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 modin/core/storage_formats/pandas/merge.py.

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.

PR Type

enhancement

Description

Added support for 'right' merge/join by swapping the left and right operands when necessary.
Introduced a flag reverted to track if the operands were swapped to ensure correct merge operations.
Adjusted the metadata computation logic to accommodate the operand swapping.
Improved performance for 'inner' merge when the left operand has a single partition.

Changes walkthrough 📝

Relevant files

Enhancement

merge.py `Implement right merge/join support with operand swapping` modin/core/storage_formats/pandas/merge.py Added logic to handle 'right' merge by swapping operands. Introduced a flag to track if operands were swapped. Adjusted metadata computation based on operand order. Ensured correct merge operation based on 'how' parameter.	+27/-4

💡 PR-Agent usage:
Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

Signed-off-by: Anatoly Myachev <anatoly.myachev@intel.com>

korbit-ai · 2024-08-15T16:01:27Z

Clone of the PR modin-project/modin#7226

korbit-ai · 2024-08-15T16:01:29Z

My review is in progress 📖 - I will have feedback for you in a few minutes!

coderabbitai · 2024-08-15T16:01:30Z

Warning

Rate limit exceeded

@furwellness has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 12 minutes and 7 seconds before requesting another review.

How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

Commits

Files that changed from the base of the PR and between b236b76 and 616b51f.

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?

Share

Tips

Chat

There are 3 ways to chat with CodeRabbit:

Review comments: Directly reply to a review comment made by CodeRabbit. Example:
- I pushed a fix in commit <commit_id>.
- Generate unit testing code for this file.
- Open a follow-up GitHub issue for this discussion.
Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
- @coderabbitai generate unit testing code for this file.
- @coderabbitai modularize this function.
PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
- @coderabbitai generate interesting stats about this repository and render them as a table.
- @coderabbitai show all the console.log statements in this repository.
- @coderabbitai read src/utils.ts and generate unit testing code.
- @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
- @coderabbitai help me debug CodeRabbit configuration file.

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)

@coderabbitai pause to pause the reviews on a PR.
@coderabbitai resume to resume the paused reviews.
@coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
@coderabbitai full review to do a full review from scratch and review all the files again.
@coderabbitai summary to regenerate the summary of the PR.
@coderabbitai resolve resolve all the CodeRabbit review comments.
@coderabbitai configuration to show the current CodeRabbit configuration for the repository.
@coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configuration File (`.coderabbit.yaml`)

You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
Please see the configuration documentation for more information.
If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

Visit our Documentation for detailed information on how to use CodeRabbit.
Join our Discord Community to get help, request features, and share feedback.
Follow us on X/Twitter for updates and announcements.

korbit-ai

I have reviewed your code and found 2 potential issues.

korbit-ai · 2024-08-15T16:14:30Z

modin/core/storage_formats/pandas/merge.py

+            if how == "inner" and left._modin_frame._partitions.shape[0] == 1:
+                left, right = right, left
+                reverted = True


The new condition to swap operands in the merge operation is a good optimization for performance. However, we need to ensure that this swap doesn't affect the order of columns in the final result. Consider adding a step after the merge to reorder the columns if the operands were swapped. This will maintain consistency with the expected output when the operands are not swapped.

Chat with Korbit by mentioning @korbit-ai, and give a 👍 or 👎 to help Korbit improve your reviews.

korbit-ai · 2024-08-15T16:14:30Z

modin/core/storage_formats/pandas/merge.py

+            if not reverted:
+                new_columns, new_dtypes = cls._compute_result_metadata(
+                    left,
+                    right,
+                    on,
+                    left_on,
+                    right_on,
+                    kwargs.get("suffixes", ("_x", "_y")),
+                )
+            else:
+                new_columns, new_dtypes = cls._compute_result_metadata(
+                    right,
+                    left,
+                    on,
+                    right_on,
+                    left_on,
+                    kwargs.get("suffixes", ("_x", "_y")),
+                )


The _compute_result_metadata method has been updated to handle the case when operands are swapped. However, it's important to ensure that this change doesn't introduce any inconsistencies in the computed metadata. Consider adding unit tests specifically for the swapped operands case to verify that the computed metadata (columns and dtypes) is correct in both swapped and non-swapped scenarios.

Chat with Korbit by mentioning @korbit-ai, and give a 👍 or 👎 to help Korbit improve your reviews.

furwellness · 2024-08-18T01:39:44Z

/review

codiumai-pr-agent-free · 2024-08-18T01:39:58Z

PR Reviewer Guide 🔍

(Review updated until commit `616b51f`)

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Key issues to review Code Duplication The code for computing result metadata is duplicated with a slight variation. Consider refactoring to avoid repetition. Performance Concern The condition for swapping left and right dataframes might not be optimal. Consider if there are other scenarios where swapping could be beneficial.

codiumai-pr-agent-free · 2024-08-20T20:46:55Z

Persistent review updated to latest commit 616b51f

furwellness · 2024-08-20T20:46:58Z

@coderabbitai full review

coderabbitai · 2024-08-20T20:47:02Z

Actions performed

Full review triggered.

codiumai-pr-agent-free · 2024-08-20T20:47:22Z

PR Code Suggestions ✨

Category	Suggestion	Score
Maintainability	Extract repeated logic into a separate method to reduce duplication Consider extracting the logic for computing result metadata into a separate method to reduce code duplication and improve maintainability. modin/core/storage_formats/pandas/merge.py [181-198] -if not reverted: - new_columns, new_dtypes = cls._compute_result_metadata( - left, - right, - on, - left_on, - right_on, - kwargs.get("suffixes", ("_x", "_y")), - ) -else: - new_columns, new_dtypes = cls._compute_result_metadata( - right, - left, - on, - right_on, - left_on, - kwargs.get("suffixes", ("_x", "_y")), +def compute_metadata(left, right, on, left_on, right_on, suffixes): + return cls._compute_result_metadata( + left, right, on, left_on, right_on, suffixes ) +suffixes = kwargs.get("suffixes", ("_x", "_y")) +if not reverted: + new_columns, new_dtypes = compute_metadata(left, right, on, left_on, right_on, suffixes) +else: + new_columns, new_dtypes = compute_metadata(right, left, on, right_on, left_on, suffixes) + Apply this suggestion Suggestion importance[1-10]: 8 Why: Extracting the logic for computing result metadata into a separate method reduces code duplication and improves maintainability, which is a significant improvement for code organization.	8
	Use a more descriptive variable name to improve code readability Consider using a more descriptive variable name instead of `reverted`. A name like `is_inner_merge_with_single_left_partition` would better explain the condition it represents. modin/core/storage_formats/pandas/merge.py [132-135] -reverted = False +is_inner_merge_with_single_left_partition = False if how == "inner" and left._modin_frame._partitions.shape[0] == 1: left, right = right, left - reverted = True + is_inner_merge_with_single_left_partition = True Apply this suggestion Suggestion importance[1-10]: 7 Why: The suggestion to use a more descriptive variable name enhances code readability and maintainability, making it easier for future developers to understand the purpose of the variable.	7
	Add a comment explaining the rationale behind swapping dataframes Consider adding a comment explaining why the `left` and `right` dataframes are swapped when `how == "inner"` and there's only one partition in the left dataframe. This would improve code clarity and maintainability. modin/core/storage_formats/pandas/merge.py [133-135] +# For inner joins, if the left dataframe has only one partition, +# we swap left and right to potentially improve performance if how == "inner" and left._modin_frame._partitions.shape[0] == 1: left, right = right, left reverted = True Apply this suggestion Suggestion importance[1-10]: 7 Why: Adding a comment to explain the logic behind swapping dataframes improves code clarity and helps maintainability by providing context for future developers.	7
Enhancement	Simplify conditional logic using a ternary operator Consider using a ternary operator to simplify the conditional logic for the pandas merge operation, making the code more concise and readable. modin/core/storage_formats/pandas/merge.py [155-158] -if reverted: - df = pandas.merge(right, left, kwargs) -else: - df = pandas.merge(left, right, kwargs) +df = pandas.merge(right, left, kwargs) if reverted else pandas.merge(left, right, kwargs) Apply this suggestion Suggestion importance[1-10]: 6 Why: Using a ternary operator simplifies the conditional logic, making the code more concise and slightly improving readability, although the improvement is minor.	6

furwellness · 2024-08-20T23:28:39Z

No comment from coderabbit

anmyachev and others added 2 commits May 10, 2024 13:31

PERF-#0000: try merge optimization

e261c83

Signed-off-by: Anatoly Myachev <anatoly.myachev@intel.com>

[skip ci]

616b51f

korbit-ai bot added the issue-evaluation label Aug 15, 2024

korbit-ai bot added the korbit-code-analysis label Aug 15, 2024

korbit-ai bot commented Aug 15, 2024

View reviewed changes

local-dev-korbit-ai-mentor bot deleted the branch main August 15, 2024 17:36

local-dev-korbit-ai-mentor bot closed this Aug 15, 2024

codiumai-pr-agent-free bot added the Review effort [1-5]: 3 label Aug 18, 2024

furwellness reopened this Aug 20, 2024

furwellness changed the base branch from cloned_main_9992b to main August 20, 2024 20:46

codiumai-pr-agent-free bot added the enhancement New feature or request label Aug 20, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

FEAT-#7254: Support right merge/join #42

FEAT-#7254: Support right merge/join #42

korbit-ai bot commented Aug 15, 2024 •

edited by codiumai-pr-agent-free bot

Loading

korbit-ai bot commented Aug 15, 2024

korbit-ai bot commented Aug 15, 2024

coderabbitai bot commented Aug 15, 2024 •

edited

Loading

Rate limit exceeded

Chat

CodeRabbit Commands (invoked as PR comments)

CodeRabbit Configuration File (`.coderabbit.yaml`)

Documentation and Community

korbit-ai bot left a comment

korbit-ai bot Aug 15, 2024

korbit-ai bot Aug 15, 2024

furwellness commented Aug 18, 2024

codiumai-pr-agent-free bot commented Aug 18, 2024 •

edited

Loading

codiumai-pr-agent-free bot commented Aug 20, 2024

furwellness commented Aug 20, 2024

coderabbitai bot commented Aug 20, 2024

codiumai-pr-agent-free bot commented Aug 20, 2024

furwellness commented Aug 20, 2024

FEAT-#7254: Support right merge/join #42

Are you sure you want to change the base?

FEAT-#7254: Support right merge/join #42

Conversation

korbit-ai bot commented Aug 15, 2024 • edited by codiumai-pr-agent-free bot Loading

User description

What do these changes do?

Description by Korbit AI

What change is being made?

Why are these changes being made?

PR Type

Description

Changes walkthrough 📝

korbit-ai bot commented Aug 15, 2024

korbit-ai bot commented Aug 15, 2024

coderabbitai bot commented Aug 15, 2024 • edited Loading

Rate limit exceeded

Chat

CodeRabbit Commands (invoked as PR comments)

CodeRabbit Configuration File (.coderabbit.yaml)

Documentation and Community

korbit-ai bot left a comment

Choose a reason for hiding this comment

korbit-ai bot Aug 15, 2024

Choose a reason for hiding this comment

korbit-ai bot Aug 15, 2024

Choose a reason for hiding this comment

furwellness commented Aug 18, 2024

codiumai-pr-agent-free bot commented Aug 18, 2024 • edited Loading

PR Reviewer Guide 🔍

(Review updated until commit 616b51f)

codiumai-pr-agent-free bot commented Aug 20, 2024

furwellness commented Aug 20, 2024

coderabbitai bot commented Aug 20, 2024

codiumai-pr-agent-free bot commented Aug 20, 2024

PR Code Suggestions ✨

furwellness commented Aug 20, 2024

korbit-ai bot commented Aug 15, 2024 •

edited by codiumai-pr-agent-free bot

Loading

coderabbitai bot commented Aug 15, 2024 •

edited

Loading

CodeRabbit Configuration File (`.coderabbit.yaml`)

codiumai-pr-agent-free bot commented Aug 18, 2024 •

edited

Loading

(Review updated until commit `616b51f`)