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.

Sign up for GitHub

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

Jump to bottom

BUG: regression in DataFrame.combine_first with integer columns (GH14687) #14886

Merged
merged 2 commits into from
Dec 16, 2016
Merged

BUG: regression in DataFrame.combine_first with integer columns (GH14687) #14886

merged 2 commits into from
Dec 16, 2016

Conversation

jorisvandenbossche
Copy link
Member

closes #14687

@jorisvandenbossche jorisvandenbossche added Bug Regression Functionality that used to work in a prior pandas version labels Dec 15, 2016
@jorisvandenbossche jorisvandenbossche added this to the 0.19.2 milestone Dec 15, 2016
jreback
jreback reviewed Dec 15, 2016
View reviewed changes
pandas/core/frame.py
otherSeries = otherSeries.astype(new_dtype)
else:
new_dtype = this_dtype
if this_dtype != other_dtype:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use not is_dtype_equal

@jreback
Copy link
Contributor

jreback commented Dec 15, 2016

minor change, lgtm otherwise.

@jorisvandenbossche
Copy link
Member Author

@sinhrks see #14687. The code I now removed was introduced in #13970 (see https://github.com/pandas-dev/pandas/pull/13970/files#diff-1e79abbbdd150d4771b91ea60a4e1cc7L3703).
Can you still remember the reason for the notnull(series).all()?

@codecov-io
Copy link

Current coverage is 85.30% (diff: 100%)

Merging #14886 into master will increase coverage by <.01%

@@             master     #14886   diff @@
==========================================
  Files           144        144          
  Lines         51004      51005     +1   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits          43511      43512     +1   
  Misses         7493       7493          
  Partials          0          0

Powered by Codecov. Last update 5f889a2...7f325cd

@sinhrks
Copy link
Member

sinhrks commented Dec 16, 2016

@jorisvandenbossche Thx for the fix.

I think it is to keep caller's dtype if it doesn't have missing. If #14687 example can keep int dtype, the logic should be unnecessary.

@jorisvandenbossche
Copy link
Member Author

I think it is to keep caller's dtype if it doesn't have missing.

OK, because there is a _possibly_downcast_to_dtype(arr, this_dtype) later on, it will be cast to int again if possible, and in this way also preserves the int, AFAIU

@jorisvandenbossche jorisvandenbossche merged commit 992dfbc into pandas-dev:master Dec 16, 2016
ischurov pushed a commit to ischurov/pandas that referenced this pull request Dec 19, 2016
@jorisvandenbossche @ischurov
BUG: regression in DataFrame.combine_first with integer columns (GH14…
6f74275 
…687) (pandas-dev#14886)
jorisvandenbossche added a commit to jorisvandenbossche/pandas that referenced this pull request Dec 24, 2016
@jorisvandenbossche
[Backport pandas-dev#14886] BUG: regression in DataFrame.combine_firs…
a509172 
…t with integer columns (GH14687) (pandas-dev#14886)

(cherry picked from commit 992dfbc)
yarikoptic added a commit to neurodebian/pandas that referenced this pull request Jan 3, 2017
@yarikoptic
Merge tag 'v0.19.2' into releases
5f6a820 
Version 0.19.2

* tag 'v0.19.2': (78 commits)
  RLS: v0.19.2
  DOC: update release notes for 0.19.2
  TST: skip gbq upload test as flakey
  DOC: clean-up v0.19.2 whatsnew
  DOC: update Pandas Cheat Sheet (GH13202)
  DOC: Pandas Cheat Sheet
  TST: matplotlib 2.0 fix in log limits for barplot (GH14808) (pandas-dev#14957)
  flake8 fix import
  Remove test - from 0.20.0 PR slipped in
  PERF: fix getitem unique_check / initialization issue
  cache and remove boxing (pandas-dev#14931)
  CLN: Resubmit of GH14700.  Fixes GH14554.  Errors other than Indexing…
  Clean up construction of Series with dictionary and datetime index
  BUG: .fillna() for datetime64 with tz is passing thru floats
  BUG: Patch read_csv NA values behaviour
  ENH: merge_asof() has type specializations and can take multiple 'by' parameters (pandas-dev#13936)
  [Backport pandas-dev#14886] BUG: regression in DataFrame.combine_first with integer columns (GH14687) (pandas-dev#14886)
  Fixed KDE Plot to drop the missing values (pandas-dev#14820)
  ENH: merge_asof() has left_index/right_index and left_by/right_by (pandas-dev#14253) (pandas-dev#14531)
  TST: correct url for test file on s3 (xref pandas-dev#14587)
  ...
yarikoptic added a commit to neurodebian/pandas that referenced this pull request Jan 3, 2017
@yarikoptic
Merge branch 'releases' into debian
f3a4da0 
* releases: (78 commits)
  RLS: v0.19.2
  DOC: update release notes for 0.19.2
  TST: skip gbq upload test as flakey
  DOC: clean-up v0.19.2 whatsnew
  DOC: update Pandas Cheat Sheet (GH13202)
  DOC: Pandas Cheat Sheet
  TST: matplotlib 2.0 fix in log limits for barplot (GH14808) (pandas-dev#14957)
  flake8 fix import
  Remove test - from 0.20.0 PR slipped in
  PERF: fix getitem unique_check / initialization issue
  cache and remove boxing (pandas-dev#14931)
  CLN: Resubmit of GH14700.  Fixes GH14554.  Errors other than Indexing…
  Clean up construction of Series with dictionary and datetime index
  BUG: .fillna() for datetime64 with tz is passing thru floats
  BUG: Patch read_csv NA values behaviour
  ENH: merge_asof() has type specializations and can take multiple 'by' parameters (pandas-dev#13936)
  [Backport pandas-dev#14886] BUG: regression in DataFrame.combine_first with integer columns (GH14687) (pandas-dev#14886)
  Fixed KDE Plot to drop the missing values (pandas-dev#14820)
  ENH: merge_asof() has left_index/right_index and left_by/right_by (pandas-dev#14253) (pandas-dev#14531)
  TST: correct url for test file on s3 (xref pandas-dev#14587)
  ...
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jreback jreback jreback approved these changes

Assignees
No one assigned
Labels
Bug Regression Functionality that used to work in a prior pandas version
Projects
None yet
Milestone
0.19.2
Development

Successfully merging this pull request may close these issues.

combine_first throws ValueError: Cannot convert NA to integer
4 participants
@jorisvandenbossche @jreback @codecov-io @sinhrks