ENH: Create merge indicator for obs from left, right, or both

[nickeubank]

xref #7412
closes #8790

Adds a new column (_merge) to DataFrames being merged that takes on a value of 1 if observation was only in left df, 2 if only in right df, and 3 if in both. Designed to address #7412 and #8790.

Update: new column now categorical with more informative labels of left_only, right_only, and both.

Still at draft stage, but want to get comments before trying to refine too much.

[jreback]

don't use inplace. It makes the code much harder to read and is not used anywhere within the pandas codebase (well it shouldn't be)

[nickeubank]

Oh! Ok will update.

@jreback : for my own purposes: I thought inplace was preferable for large
datasets because it prevents unnecessary memory duplication. Am I wrong
that:

s = s.fillna(0)

Causes duplication of s in memory (at least temporarily) while:

s.fillna(0, inplace=True)

Does not?
On Mon, May 4, 2015 at 3:37 AM jreback [email protected] wrote:

In pandas/tools/merge.py
#10054 (comment):

•    columns = left.columns.values.tolist() + right.columns.values.tolist()
  

•    for i in ['_left_indicator', '_right_indicator', '_merge']:
  

•        if i in columns:
  

•            raise ValueError("Cannot use `indicator=True` option when data contains a column named {}".format(i))
  

•    left['_left_indicator'] = 1
  

•    left['_left_indicator'] = left['_left_indicator'].astype('int8')
  

•    right['_right_indicator'] = 2
  

•    right['_right_indicator'] = right['_right_indicator'].astype('int8')
  
• def _indicator_post_merge(self, result, left, right):

•    result['_left_indicator'].fillna(0, inplace=True)
  

don't use inplace. It makes the code much harder to read and is not used
anywhere within the pandas codebase (well it shouldn't be)

—
Reply to this email directly or view it on GitHub
https://github.com/pydata/pandas/pull/10054/files#r29576706.

[nickeubank]

@jreback : I've pushed a version that does not use inplace, but it comes at a cost. Namely, I don't think I can avoid making a full duplicates of both input DataFrames if I don't use inplace=True for the drop() commands.

The difficulty is the following: at the moment I'm adding indicator columns to the left and right DataFrames (which I can do without inplace), passing the updated DataFrames into the concatenate_block_managers, then using those columns post-merge to identify the source of each row.

This can be done in one of two ways: (1) modify the input DataFrames by adding new column to those objects and then deleting those columns after the merge (as I do in commit 76e4f33), or (2) duplicate those objects and then add the columns to the copies of the original DataFrames (as in this commit, cf8412f).

(1) seems preferable to me since it does not entail the duplication of both input DataFrames in memory. But I think (1) requires inplace modification to remove those indicator columns.

With inplace=True, I can cleanup in _indicator_post_merge() as follows:

left.drop(label='_left_indicator', axis=1, inplace=True)

But if I do:

left = left.drop(label='_left_indicator', axis=1)

then that's not actually changing the input DataFrame, just creating a new DataFrame and pointing the variable left to it. But once the function finishes, since merge doesn't return new versions of the input DataFrames, those changes disappear, and the input DataFrames still have the extra columns.

So I think the question is: can we make an exception to the "no use of inplace rule" to allow this option to not fully duplicate both input DataFrames?

[jreback]

@nickeubank inplace ops, rarely are faster. They most often copy, modify the copy, the make it so the original variable points to the copy.

The semantics of almost all pandas methods (e.g. everything except in-place indexing / inplace kw), are NOT to touch the passed in data in any way.

If you want an indicator for the merged data, I dont' see a problem with simply copying the input data, then do what you want.

The marginal perf benefit is really small. We prefer correctness over perf anyday.

[nickeubank]

OK, thanks for clarifying! Go to know both that that's how it works, and your preference ordering. :)

[nickeubank]

@jreback One other question on priorities: I thought of a way to do this that would be slower than the version I've pushed, but would require less memory. Any rules of thumb for balancing those interests?

Currently: Copy input data; add indicator columns; feed through merge; create an indicator of where each column came from based on those indicator columns.
-> One merge, but have to make copies of both input DataFrames.

Alternative: Merge input data as normal; pull out merging columns from input data and merge them into the output, use those merges to create indicators for where each row came from.
-> Three merges (initial, and one for each input), but no full copying of input DataFrames.

[jreback]

just copy the data much simpler

[nickeubank]

Added docs. Anything else?

[jreback]

you can move this to the 0.17.0 docs

[jreback]

@nickeubank can you rebase this and i'll take another look

[nickeubank]

@jreback rebased!

[jreback]

add the issue number here

[nickeubank]

@jreback added suggested tweak to allow users to pass strings to indicator as name for indicator column.

[jreback]

add a versionadded directive

[nickeubank]

Check (I think that's formatted correctly?)

[nickeubank]

@jreback sorry about scrubbing confusion. Think better now.

[jreback]

add a version added here as well

[jreback]

ok, code looks good. I like what you added to merging.rst, can you put in bullet points or a table-like for better readability (the the meaning of the returned values in the merge indicator)

[nickeubank]

@jreback ok, updated.

One note: not quite sure how best to format the versionadded advisory in the list of options for merge -- made an attempt but don't love it. Let me know if there's a better way.

[jreback]

look at how changes in convert_objects is done in whatsnew/v0.17.0 (a table is created its pretty easy)

[nickeubank]

ok -- now a table!

[jreback]

right_only

[jreback]

can you add some tests with multiple merge keys (e.g. a list for on). seems should still work.

[nickeubank]

@jreback added.

Sidenote: I hadn't noticed this before, but merge type-coerces key columns from int to float if you do an outer merge, even though the output is all int. Is that a known behavior?:

In[30]:
df1 = pd.DataFrame({'col1':[0,1], 'colleft':['a','b']})
df2 = pd.DataFrame({'col1':[1,3], 'colright':['b','x']})
pd.merge(df1, df2, how='outer', on='col1').col1.dtypes
Out[30]: dtype('float64')

In[31]:
pd.merge(df1, df2, how='inner', on='col1').col1.dtypes
Out[31]: dtype('int64')    

[jreback]

@nickeubank this is a byproduct of that a-prior you don't know if you will have to put NaN in columns, so you just coerce to a nan-capable type then go from there. I suppose you could in this case coerce back.

If you'd like to make another issue, go ahead. (though look thru existing ones as it might already be there). and of course PR's are welcome.

[jreback]

you can put that mini-table here as well

[nickeubank]

@jreback table added!

thanks for note about coercion. Doesn't seem like highest priority, but I'll keep it in mind for a place to contribute later!

[nickeubank]

@jreback Anything else you'd like me to amend on this?

[jreback]

wanted @jorisvandenbossche to have a look

[jreback]

@nickeubank pls rebase

@TomAugspurger @jorisvandenbossche any comments?

[nickeubank]

@jreback rebased

[jreback]

@shoyer quick look pls.

[shoyer]

This looks pretty clean to me. Nice work @nickeubank!

[jreback]

thanks!