ENH: Implement cross method for Merge Operations #37864

phofl · 2020-11-15T16:35:06Z

closes ENH: cross join in merge() and join() #5401
tests added / passed
passes black pandas
passes git diff upstream/master -u -- "*.py" | flake8 --diff
whatsnew entry

This is a first draft of a cross method for merge operations. As suggested by @jreback I added a new column and dispatched to inner as a first naive implementation. I am currently wondering, if there would be a better place to adjust the arguments than the places I selected. I wanted to avoid to modify self in some method called within the constructor, so i modified the inputs before adding the parameters to self. This currently does not support the cross method for join. If we get a consensus where to put the modifications, I would add support for join. Also have to add more tests probably and something to the userguide

Have to look into the performance afterwards. Copies of the Left and Right frames may be a big performance hit for bigger frames?

jreback

not bad, can you add some asv's as well

pandas/core/frame.py

jreback · 2020-11-18T13:41:30Z

pandas/core/reshape/merge.py

@@ -591,6 +591,8 @@ def __init__(
    ):
        _left = _validate_operand(left)
        _right = _validate_operand(right)
+        if how == "cross":
+            _left, _right, how, on = self._create_cross_configuration(_left, _right, on)


return the new column here (in addition to the other values)

jreback · 2020-11-18T13:41:41Z

pandas/core/reshape/merge.py

        return result.__finalize__(self, method="merge")

+    def _maybe_drop_cross_column(self, result: "DataFrame"):


pass the col in here (type the output)

pandas/core/reshape/merge.py

jreback · 2020-11-18T13:45:37Z

pandas/core/reshape/merge.py

+    def _create_cross_configuration(
+        self, _left, _right, on
+    ) -> Tuple["DataFrame", "DataFrame", str, str]:
+        if on is not None:


this validation should be done first (IOW maybe move create_cross_configuration lower where you are calling)

Just to be sure, that I understand you correctly: Calling the function _create_cross_configuration should be done after calling _validate_specification? in this case I have moved the parts around

jreback · 2020-11-18T13:46:13Z

pandas/core/reshape/merge.py

+        cross_col = f"{max([*_left.columns, *_right.columns])}_cross"
+        _left = _left.copy()
+        _right = _right.copy()
+        _left.insert(loc=0, value=1, column=cross_col)


you can just use .assign (IOW put it at the end)

jreback · 2020-11-18T13:47:12Z

pandas/core/reshape/merge.py

    def _validate_specification(self):
+        if hasattr(self, "_cross"):


instead of checking the attribute, rather can you check if how is 'cross'?

Depending on answer above, this is done

pandas/tests/reshape/merge/test_merge.py

phofl · 2020-11-18T14:13:38Z

Thx for the feedback, will start to implement the changes in the evening probably

pep8speaks · 2020-11-18T23:12:00Z

Hello @phofl! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2020-11-25 22:31:42 UTC

pandas/core/frame.py

pandas/core/reshape/merge.py

jreback · 2020-11-19T18:44:27Z

pandas/core/reshape/merge.py

+                or self.on is not None
+            ):
+                raise MergeError(
+                    "Can not pass any merge columns when using cross as merge method"


can you say that left_on,right_on,on must be None, and left_index,right_index must be False

jreback

some comments, pls also add all examples from the OP (hopefully was more than one).

also might be some examples on SO can add as tests.

jreback · 2020-11-21T01:04:14Z

pandas/core/frame.py

@@ -221,6 +223,11 @@
      join; sort keys lexicographically.
    * inner: use intersection of keys from both frames, similar to a SQL inner
      join; preserve the order of the left keys.
+    * cross: creates the karthesian product from both frames, preserves the order


German influence, sorry :)

jreback · 2020-11-21T01:05:10Z

pandas/core/frame.py

@@ -341,6 +348,24 @@
 ...
 ValueError: columns overlap but no suffix specified:
    Index(['value'], dtype='object')
+
+>>> df1 = pd.DataFrame({'left': ['foo', 'bar']})


can you add an example of an inner and left merge here (and put them right before this)

Done, could you please check if this is similar to what you have in mind?

jreback · 2020-11-21T01:05:55Z

pandas/core/reshape/merge.py

@@ -1200,9 +1218,50 @@ def _maybe_coerce_merge_keys(self):
                typ = rk.categories.dtype if rk_is_cat else object
                self.right = self.right.assign(**{name: self.right[name].astype(typ)})

+    def _create_cross_configuration(
+        self, _left, _right


name these left, right

pandas/tests/reshape/merge/test_merge.py

phofl · 2020-11-22T00:15:16Z

I added a few tests covering mixed dtypes, nulls, more columns and different lengths

jreback

small requests, ping on green

jreback · 2020-11-23T13:42:50Z

pandas/core/reshape/merge.py

+
+        Parameters
+        ----------
+        _left: DataFrame


can you match the signature

jreback · 2020-11-23T13:42:55Z

pandas/core/reshape/merge.py

+
+        Returns
+        -------
+            a tuple (_left_df, _right_df, how, cross_col) representing the adjusted


jreback · 2020-11-23T13:43:54Z

pandas/tests/reshape/merge/test_join.py

@@ -803,3 +805,27 @@ def test_join_inner_multiindex_deterministic_order():
        index=MultiIndex.from_tuples([(2, 1, 4, 3)], names=("b", "a", "d", "c")),
    )
    tm.assert_frame_equal(result, expected)
+


can you moe to test_cross_merge (same dir)

If I understand you correctly, it is directly below the other cross tests

phofl · 2020-11-23T23:26:30Z

@jreback greenish. Failure unrelated

jreback · 2020-11-23T23:52:47Z

pandas/tests/reshape/merge/test_merge.py

+
+
+@pytest.mark.parametrize(
+    ("input_col", "output_cols"), [("b", ["a", "b"]), ("a", ["a_x", "a_y"])]


sorry maybe i wasn't clear, can you make a new file called test_merge_cross.py and put these there.

Aaah ok, created the file and moved the tests

jreback

one more case

if on is a duplicate column what happens? (raising is ok)

phofl · 2020-11-24T18:31:37Z

Good point, deleted the column before. Now we are raising a MergeError

jreback · 2020-11-24T21:58:00Z

pandas/tests/reshape/merge/test_merge_cross.py

+        left.join(right, how="cross", on="a")
+
+
+def test_merge_cross_duplicate_on_column():


didn't mean this case (I don't think this is actually possible to happen).

I mean what if you have an input like

left=pd.DataFrame(['a': [1, 2], 'b': [3, 4]}) pd.merge(left, left, how='cross', on=['a', 'a'])

Could happen if we are really really unlucky :)

We do not allow on columns in case of cross, so we are safe with this

Should I reverse my change then?

yeah revert this change (its not worth checking)

Done, will ping when green

jreback · 2020-11-25T20:55:31Z

ok i think your version before the last commit is good. ping on green.

This reverts commit 4589651

� Conflicts: � doc/source/whatsnew/v1.2.0.rst

jreback · 2020-11-26T00:31:51Z

thanks @phofl very nice!

Coldsp33d · 2020-11-26T07:24:03Z

Great job, and thanks for implementing this! I've given your solution the advertisement it deserves on stack.

Excited to see this in 1.2.

jreback · 2020-11-26T13:12:52Z

@phofl created an issue for benchmarks

it might make sense to change to use a broadcasting impl (like refs on SO) or use pandas cartesian product function - if the benchmarks show this - can create an issue or better to do benchmarks first

phofl · 2020-11-26T13:43:07Z

@jreback If you are referring to asvs, I have added two benchmarks through this pr

jreback · 2020-11-26T13:47:47Z

oh right u did! ok

phofl · 2020-11-26T21:12:43Z

Will try to look into the numpy or cartesian function when I've got a bit more time uninterrupted

phofl added 2 commits November 15, 2020 17:28

First cross merge draft for merge operation

71edcce

Merge branch 'master' of https://github.com/pandas-dev/pandas

cc5d779

phofl marked this pull request as draft November 15, 2020 16:35

Fix variable assignment

f573ca4

jreback requested changes Nov 18, 2020

View reviewed changes

pandas/tests/reshape/merge/test_merge.py Outdated Show resolved Hide resolved

jreback added Reshaping Concat, Merge/Join, Stack/Unstack, Explode Enhancement labels Nov 18, 2020

phofl added 9 commits November 18, 2020 23:19

Adress review comments

0acdd00

Change function signature

949185e

Add cross functionality for join

2d5ccaa

Change docs

9098852

Add asvs

60f6b25

Move import

0601243

Assign value

c46eab3

Reduce asvs

6274120

Merge branch 'master' of https://github.com/pandas-dev/pandas into 5401

891785d

Remove whitespaces

1f0a1c8

phofl force-pushed the 5401 branch from 9cf5d59 to 891785d Compare November 18, 2020 23:16

Move import

d7c1156

jreback requested changes Nov 19, 2020

View reviewed changes

phofl added 4 commits November 19, 2020 20:00

Adress review

7c8d37a

Fix doc checks

651540e

Add docstring

f7cdd4d

Change example

a4d24a9

phofl marked this pull request as ready for review November 21, 2020 00:09

jreback added this to the 1.2 milestone Nov 21, 2020

jreback requested changes Nov 21, 2020

View reviewed changes

Add tests

94b1367

Fix doc

77a9e23

jreback requested changes Nov 23, 2020

View reviewed changes

phofl added 3 commits November 23, 2020 22:11

Change signature

4597642

Move test

67a67a6

Delete import

f731081

Create new file

4fcde78

jreback approved these changes Nov 23, 2020

View reviewed changes

jreback requested changes Nov 23, 2020

View reviewed changes

jreback reviewed Nov 24, 2020

View reviewed changes

Raise if duplicate on column

4589651

jreback requested changes Nov 24, 2020

View reviewed changes

phofl added 2 commits November 25, 2020 22:23

Revert "Raise if duplicate on column"

d964ef1

This reverts commit 4589651

Merge branch 'master' of https://github.com/pandas-dev/pandas into 5401

a1eeaa4

� Conflicts: � doc/source/whatsnew/v1.2.0.rst

jreback approved these changes Nov 26, 2020

View reviewed changes

jreback merged commit d8a2e74 into pandas-dev:master Nov 26, 2020

jreback mentioned this pull request Nov 26, 2020

PERF: investigate alt implementations for cross join #38082

Open

phofl deleted the 5401 branch November 26, 2020 13:43

jbrockmendel pushed a commit to jbrockmendel/pandas that referenced this pull request Nov 27, 2020

ENH: Implement cross method for Merge Operations (pandas-dev#37864)

47fea62

mroeschke mentioned this pull request Aug 10, 2022

BUG: use uuid4 instead of md5 for fake column in cross merge #48029

Merged

5 tasks

		return result.__finalize__(self, method="merge")

		def _maybe_drop_cross_column(self, result: "DataFrame"):

		def _validate_specification(self):
		if hasattr(self, "_cross"):



		@pytest.mark.parametrize(
		("input_col", "output_cols"), [("b", ["a", "b"]), ("a", ["a_x", "a_y"])]

		left.join(right, how="cross", on="a")


		def test_merge_cross_duplicate_on_column():

ENH: Implement cross method for Merge Operations #37864

ENH: Implement cross method for Merge Operations #37864

Conversation

phofl commented Nov 15, 2020 • edited by jreback Loading

jreback left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

phofl commented Nov 18, 2020

pep8speaks commented Nov 18, 2020 • edited Loading

Comment last updated at 2020-11-25 22:31:42 UTC

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jreback left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

phofl commented Nov 22, 2020

jreback left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

phofl commented Nov 23, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jreback left a comment

Choose a reason for hiding this comment

phofl commented Nov 24, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jreback commented Nov 25, 2020

jreback commented Nov 26, 2020

Coldsp33d commented Nov 26, 2020

jreback commented Nov 26, 2020

phofl commented Nov 26, 2020 • edited Loading

jreback commented Nov 26, 2020

phofl commented Nov 26, 2020

phofl commented Nov 15, 2020 •

edited by jreback

Loading

pep8speaks commented Nov 18, 2020 •

edited

Loading

phofl commented Nov 24, 2020 •

edited

Loading

phofl commented Nov 26, 2020 •

edited

Loading