Skip to content

BUG: Allow multiple 'by' parameters in merge_asof() when DataFrames are indexed (#15676) #15679

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
Closed
wants to merge 2 commits into from
Closed

BUG: Allow multiple 'by' parameters in merge_asof() when DataFrames are indexed (#15676) #15679

wants to merge 2 commits into from

Conversation

chrisaycock
Copy link
Contributor

@codecov
Copy link

codecov bot commented Mar 14, 2017 •
edited
Loading

Codecov Report

Merging #15679 into master will decrease coverage by 0.02%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master   #15679      +/-   ##
==========================================
- Coverage   91.03%      91%   -0.03%     
==========================================
  Files         143      143              
  Lines       49392    49399       +7     
==========================================
- Hits        44962    44958       -4     
- Misses       4430     4441      +11
Impacted Files Coverage Δ
pandas/tools/merge.py 93.68% <100%> (-0.27%) ❌
pandas/io/gbq.py 25% <0%> (-58.34%) ❌
pandas/core/frame.py 97.87% <0%> (-0.1%) ❌
pandas/tseries/index.py 95.29% <0%> (-0.1%) ❌
pandas/core/common.py 91.3% <0%> (+0.33%) ✅

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 05d70f4...965caf2. Read the comment docs.

@jreback jreback added Bug Reshaping Concat, Merge/Join, Stack/Unstack, Explode labels Mar 14, 2017
jreback
jreback reviewed Mar 14, 2017
View reviewed changes
pandas/tools/merge.py
@@ -1264,13 +1264,21 @@ def flip(xs):

# a "by" parameter requires special handling
if self.left_by is not None:
Copy link
Contributor

Choose a reason for hiding this comment

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

is this also changed for right_by? any way to make this a touch more DRY. (don't spend a lot of time as this is pretty complicated, but any simplification would be great).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I guess I could leave a comment since it also affects the new line 1276. Basically, the left and right "by" parameters have to be the same length. (I should also add verification of this starting at line 1157.) Because they are the same length, checking for one means I can assume the other. That's why I only bother with a left conditional.

@jreback jreback added this to the 0.20.0 milestone Mar 14, 2017
@jreback
Copy link
Contributor

jreback commented Mar 14, 2017

lgtm. ping on green.

@chrisaycock
Copy link
Contributor Author

@jreback It's all green!

@jreback jreback closed this in 2621b31 Mar 14, 2017
@jreback
Copy link
Contributor

jreback commented Mar 14, 2017

thanks @chrisaycock very nice!

@chrisaycock chrisaycock deleted the GH15676 branch March 14, 2017 14:11
AnkurDedania pushed a commit to AnkurDedania/pandas that referenced this pull request Mar 21, 2017
@AnkurDedania
BUG: Allow multiple 'by' parameters in merge_asof() when DataFrames a…
8f410a4 
…re indexed (pandas-dev#15676)

closes pandas-dev#15676

Author: Christopher C. Aycock <[email protected]>

Closes pandas-dev#15679 from chrisaycock/GH15676 and squashes the following commits:

965caf2 [Christopher C. Aycock] Verify that 'by' parameters are the same length
4a2cc09 [Christopher C. Aycock] BUG: Allow multiple 'by' parameters in merge_asof() when DataFrames are indexed (pandas-dev#15676)
mattip pushed a commit to mattip/pandas that referenced this pull request Apr 3, 2017
@mattip
BUG: Allow multiple 'by' parameters in merge_asof() when DataFrames a…
793e262 
…re indexed (pandas-dev#15676)

closes pandas-dev#15676

Author: Christopher C. Aycock <[email protected]>

Closes pandas-dev#15679 from chrisaycock/GH15676 and squashes the following commits:

965caf2 [Christopher C. Aycock] Verify that 'by' parameters are the same length
4a2cc09 [Christopher C. Aycock] BUG: Allow multiple 'by' parameters in merge_asof() when DataFrames are indexed (pandas-dev#15676)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jreback jreback jreback left review comments

Assignees
No one assigned
Labels
Bug Reshaping Concat, Merge/Join, Stack/Unstack, Explode
Projects
None yet
Milestone
0.20.0
Development

Successfully merging this pull request may close these issues.

BUG: merge_asof only merges by first column of 'by=' list
2 participants
@chrisaycock @jreback