BUG: Keep float dtype in merge on int and float column #18352

reidy-p · 2017-11-18T00:47:22Z

closes merge of int and float column results in column of dtype object #16572
tests added / passed
passes git diff upstream/master -u -- "*.py" | flake8 --diff
whatsnew entry

jreback · 2017-11-18T00:51:18Z

pandas/tests/reshape/test_merge.py

+        B = DataFrame({'Y': float_vals})
+
+        res = A.merge(B, left_on='X', right_on='Y')
+        assert is_float_dtype(res['Y'].dtype)


you need to check the results
the last merge is not what you would expect

On my branch:

In [1]: A = pd.DataFrame({'X': [1, 2, 3]}) In [2]: B = pd.DataFrame({'Y': [1.1, 2.5, 3.0]}) In [3]: A.merge(B, left_on='X', right_on='Y') Out[3]: X Y 0 3 3.0 In [4]: A.merge(B, left_on='X', right_on='Y').dtypes Out[4]: X int64 Y float64 dtype: object

Is this not what we would expect?

And could you give me an example of the problems caused when merging integer and float columns when the float values are not equal to the int representation of those same values? Thanks.

hmm that looks ok, in any event compare the expected results (not the dtypes)

the issue with comparing floats and integers is almost always wrong, IOW the user needs to be aware of this, as its almost always an error (so warning is ok here).

codecov · 2017-11-18T03:06:57Z

Codecov Report

Merging #18352 into master will decrease coverage by 0.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master   #18352      +/-   ##
==========================================
- Coverage   91.35%   91.34%   -0.02%     
==========================================
  Files         163      163              
  Lines       49691    49697       +6     
==========================================
- Hits        45397    45394       -3     
- Misses       4294     4303       +9

Flag	Coverage Δ
#multiple	`89.14% <100%> (ø)`	⬆️
#single	`39.67% <0%> (-0.07%)`	⬇️

Impacted Files	Coverage Δ
pandas/core/reshape/merge.py	`94.32% <100%> (+0.05%)`	⬆️
pandas/io/gbq.py	`25% <0%> (-58.34%)`	⬇️
pandas/core/frame.py	`97.8% <0%> (-0.1%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fedc503...a3d6fe6. Read the comment docs.

jreback

this needs a sub-section in other enhancements

jreback · 2017-11-19T16:18:24Z

pandas/core/reshape/merge.py

            # further if we are object, but we infer to
            # the same, then proceed
            if is_numeric_dtype(lk) and is_numeric_dtype(rk):
                if lk.dtype.kind == rk.dtype.kind:
                    continue

+                # check whether ints and floats
+                if is_integer_dtype(rk) and is_float_dtype(lk):


I am a little leary of merging integer & float columns when the floats are not actually == ints. A warning would be good here if

not (lk == lk.astype(rk)).all() is True (and the reverse for 2nd case)

jreback · 2017-11-19T16:18:47Z

pandas/core/reshape/merge.py

@@ -906,13 +906,20 @@ def _maybe_coerce_merge_keys(self):
                continue

            # if we are numeric, then allow differing
-            # kinds to proceed, eg. int64 and int8
+            # kinds to proceed, eg. int64 and int8, int and float
            # further if we are object, but we infer to
            # the same, then proceed
            if is_numeric_dtype(lk) and is_numeric_dtype(rk):
                if lk.dtype.kind == rk.dtype.kind:
                    continue



also, let's change this to an if/elsif/else clause (and use pass rather than continue to delineate the cases)

jreback · 2017-11-22T02:08:03Z

pandas/core/reshape/merge.py

                    continue

+                else:


oh I meant put the 'houston we have a problem' in the else. then everything else is just a pass.

jreback · 2017-11-22T02:09:40Z

pandas/tests/reshape/test_merge.py

+        B = DataFrame({'Y': float_vals})
+
+        res = A.merge(B, left_on='X', right_on='Y')
+        assert is_float_dtype(res['Y'].dtype)


hmm that looks ok, in any event compare the expected results (not the dtypes)

jreback · 2017-11-22T02:11:00Z

pandas/tests/reshape/test_merge.py

+        B = DataFrame({'Y': float_vals})
+
+        res = A.merge(B, left_on='X', right_on='Y')
+        assert is_float_dtype(res['Y'].dtype)


the issue with comparing floats and integers is almost always wrong, IOW the user needs to be aware of this, as its almost always an error (so warning is ok here).

jreback

minor comments. ping on green.

jreback · 2017-11-22T22:54:45Z

pandas/core/reshape/merge.py

+                                      'columns where the float values '
+                                      'are not equal to their int '
+                                      'representation', UserWarning)
+                    pass


you don't actually need the pass here (as you have the if statement)

jreback · 2017-11-22T22:54:46Z

pandas/core/reshape/merge.py

+                                      'columns where the float values '
+                                      'are not equal to their int '
+                                      'representation', UserWarning)
+                    pass


jreback · 2017-11-22T22:54:49Z

pandas/core/reshape/merge.py

-                if lib.infer_dtype(lk) == lib.infer_dtype(rk):
-                    continue
+                elif lib.infer_dtype(lk) == lib.infer_dtype(rk):
+                    pass


this ones good!

jreback · 2017-11-22T22:55:23Z

doc/source/whatsnew/v0.22.0.txt

@@ -29,6 +29,7 @@ Other Enhancements
 - :class:`pandas.io.formats.style.Styler` now has method ``hide_columns()`` to determine whether columns will be hidden in output (:issue:`14194`)
 - Improved wording of ``ValueError`` raised in :func:`to_datetime` when ``unit=`` is passed with a non-convertible value (:issue:`14350`)
 - :func:`Series.fillna` now accepts a Series or a dict as a ``value`` for a categorical dtype (:issue:`17033`)
+- :func:`pandas.DataFrame.merge` no longer casts a ``float`` column to ``object`` when merging on ``int`` and ``float`` columns (:issue:`16572`)


move to api_breaking

jreback · 2017-11-22T22:55:49Z

pandas/tests/reshape/test_merge.py

+        # merging on float and int columns
+        A = DataFrame({'X': int_vals})
+        B = DataFrame({'Y': float_vals})
+        exp_res = DataFrame(exp_vals)


call this expected

jreback · 2017-11-22T22:55:57Z

pandas/tests/reshape/test_merge.py

+        res = A.merge(B, left_on='X', right_on='Y')
+        assert_frame_equal(res, exp_res)
+
+        res = B.merge(A, left_on='Y', right_on='X')


call these result

jreback · 2017-11-22T22:56:04Z

pandas/tests/reshape/test_merge.py

+        # equal to their int representation
+        A = DataFrame({'X': [1, 2, 3]})
+        B = DataFrame({'Y': [1.1, 2.5, 3.0]})
+        exp_res = DataFrame({'X': [3], 'Y': [3.0]})


same as above

reidy-p · 2017-11-23T09:13:27Z

@jreback thanks for the comments. Green now.

jreback · 2017-11-23T15:56:18Z

thanks @reidy-p nice patch and great responsiveness!

jreback requested changes Nov 18, 2017

View reviewed changes

jreback requested changes Nov 19, 2017

View reviewed changes

jreback added Dtype Conversions Unexpected or buggy dtype conversions Reshaping Concat, Merge/Join, Stack/Unstack, Explode labels Nov 19, 2017

reidy-p force-pushed the merge_int_float branch from e6469d5 to eb511a0 Compare November 20, 2017 23:44

jreback requested changes Nov 22, 2017

View reviewed changes

reidy-p force-pushed the merge_int_float branch from e464769 to 2b8190d Compare November 22, 2017 20:24

jreback requested changes Nov 22, 2017

View reviewed changes

reidy-p added 8 commits November 22, 2017 23:29

BUG: Keep float dtype in merge on int and float column

5a0b8f1

check if float values equal to int representation and add warning

039eeae

whatsnew

7f1db98

test for merge results

9c3aea2

add else statement

d86c1fb

update whatsnew and tests

fc8ac2c

remove some pass statements

c46875b

whitespace

a3d6fe6

reidy-p force-pushed the merge_int_float branch from 2b8190d to a3d6fe6 Compare November 22, 2017 23:59

jreback added this to the 0.22.0 milestone Nov 23, 2017

jreback approved these changes Nov 23, 2017

View reviewed changes

jreback merged commit 4e98a7b into pandas-dev:master Nov 23, 2017

reidy-p deleted the merge_int_float branch November 27, 2017 14:50

jorisvandenbossche mentioned this pull request May 10, 2018

Merging on int / float columns with NaNs gives spurious warning #20998

Closed

jorisvandenbossche mentioned this pull request May 24, 2018

Value error using sjoin with pandas v0.23 geopandas/geopandas#731

Closed

rbenes mentioned this pull request Feb 25, 2019

pd.merge() on int+float produces object since 0.20 #18302

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

BUG: Keep float dtype in merge on int and float column #18352

BUG: Keep float dtype in merge on int and float column #18352

reidy-p commented Nov 18, 2017 •

edited

Loading

jreback Nov 18, 2017

reidy-p Nov 20, 2017 •

edited

Loading

jreback Nov 22, 2017

jreback Nov 22, 2017

codecov bot commented Nov 18, 2017 •

edited

Loading

jreback left a comment

jreback Nov 19, 2017

jreback Nov 19, 2017

jreback Nov 22, 2017

jreback Nov 22, 2017

jreback Nov 22, 2017

jreback left a comment

jreback Nov 22, 2017

jreback Nov 22, 2017

jreback Nov 22, 2017

jreback Nov 22, 2017

jreback Nov 22, 2017

jreback Nov 22, 2017

jreback Nov 22, 2017

reidy-p commented Nov 23, 2017

jreback commented Nov 23, 2017

BUG: Keep float dtype in merge on int and float column #18352

BUG: Keep float dtype in merge on int and float column #18352

Conversation

reidy-p commented Nov 18, 2017 • edited Loading

Choose a reason for hiding this comment

reidy-p Nov 20, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Nov 18, 2017 • edited Loading

Codecov Report

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

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

reidy-p commented Nov 23, 2017

jreback commented Nov 23, 2017

reidy-p commented Nov 18, 2017 •

edited

Loading

reidy-p Nov 20, 2017 •

edited

Loading

codecov bot commented Nov 18, 2017 •

edited

Loading