Skip to content
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

Wrong dtypes after merge op #7212

Open
anmyachev opened this issue Apr 23, 2024 · 2 comments
Open

Wrong dtypes after merge op #7212

anmyachev opened this issue Apr 23, 2024 · 2 comments
Labels
bug 🦗 Something isn't working P1 Important tasks that we should complete soon

Comments

@anmyachev
Copy link
Collaborator

import ray
ray.init()
import modin.pandas as pd

left = pd.DataFrame({"a": [1,2,3], "b": [4,5,6]})
right = pd.DataFrame({"a": [1,2,4], "c": [4,2,6]})
res = left.merge(right, on="a", how="left")

dtypes = res.dtypes
assert dtypes["a"] == "int64"
assert dtypes["b"] == "int64"
assert dtypes["c"] == "float64", f"{str(dtypes['c'])} != float64"  # <- failed

Checked on 3abd961.

@anmyachev anmyachev added bug 🦗 Something isn't working P1 Important tasks that we should complete soon labels Apr 23, 2024
@YarShev
Copy link
Collaborator

YarShev commented Apr 23, 2024

It seems we can't know new_dtypes for columns other than on in advance. @dchigarev, do you think we should change the logic in _compute_result_metadata to only compute dtypes for on or we should completely remove the logic for new_dtypes computation?

def _compute_result_metadata(cls, left, right, on, left_on, right_on, suffixes):

@dchigarev
Copy link
Collaborator

dchigarev commented Apr 24, 2024

It seems we can't know new_dtypes for columns other than on in advance.

I believe in case of a left-merge we can preserve dtypes for 'on' columns and all the columns from the left frame (since the left frame is never reindexed as far as I understand). We can also keep dtypes for the 'object' columns of the right frame, since in this case the column's dtype won't change even if we add NaNs to it.

For an inner merge, it seems that the current logic of dtypes precomputation works just fine (since we don't add NaNs to columns).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🦗 Something isn't working P1 Important tasks that we should complete soon
Projects
None yet
Development

No branches or pull requests

3 participants