API: Allow use of keep="all" in duplicated

[svenharris]

keep="all" as parameter option in duplicated

My view is that a more intuitive parametrization of the duplicated function would be that keep='all' should allow you to keep all, since keep=False kind of doesn't make any sense.

I have started on a pull request but had not realised how tightly coupled the drop_duplicates and duplicated functions are (but it makes sense of course) - I guess this is why the choice of keep=False was made in the first place.

I am of the view that keep="all" in the drop_duplicates method should raise an error, because again, that would be counter-intuitive behaviour.

[mroeschke]

So keep='all' would be the same as keep=False?

The keep='all' ("keep all the duplicates") sounds likes it's giving the opposite message as keep=False ("keep none of the duplicates"). I think its also important for duplicated to have the same argument options as drop_duplicates.

[svenharris]

Yeah I agree with the point that keep=all and keep=False sound opposite, that's the grounding of me raising this issue because I don't think keep=False sounds meaningful for the duplicated function (when you actually want to keep all). It only makes sense in the drop_duplicates function when you don't want to keep any

[mroeschke]

I am guessing the keep arguments are the same between both to keep synergy between duplicated and drop_duplicates.

duplicated is usually used as a mask, so I think the train of thought is "I don't want to keep duplicates, so mark all of them as True"

Overall though I am -1 of having two keyword arguments have the same effect, but I am open to opinions of other users and core devs.

[Liam3851]

@mroeschke As a user I tend to agree with Sven: if I'm dropping duplicates and happy with that, I use the drop_duplicates function. If I'm using duplicated directly, it's almost always that I'm in a data-cleaning/debugging exercise and the point is to use duplicated in order to find (and keep only!) the actual duplicate entries, e.g.

df = ...
surprise_data_to_check_for_errors = df.loc[df.duplicated(keep=False, subset=cols_to_check)]

and I have always found the use of keep=False in duplicated counterintuitive for this use case to say the least.

Because drop_duplicates exists I'm not sure how comparatively common would be user code that would take the mask from duplicated in order to delete it, i.e. why write the following triple-negative:

df = df.loc[~df.duplicated(keep=False)]

when you could just write

df = df.drop_duplicates(keep=False)

So as a result, does it really matter if the two methods have the same signature?

If they can have different signatures, I'd be +1 on replacing keep=False in duplicated to keep='all' and dropping the former (with a deprecation cycle of course), while leaving drop_duplicates the same. Or, for that matter, renaming the keep parameter in duplicated entirely (e.g. to mark or some such, since we're making a boolean mask, not frankly keeping or dropping anything) while leaving the signature of drop_duplicates the same.

[jorisvandenbossche]

Yeah I agree with the point that keep=all and keep=False sound opposite, that's the grounding of me raising this issue because I don't think keep=False sounds meaningful

@svenharris So it is still not fully clear to me from your explanation here, but it seems you want to add keep='all' as a more meaningful alias for keep=False (which seems to be confirmed by looking at your PR), is that correct?

So I agree that the keep=False is a confusing keyword (and also somewhat inconsistent that the argument mixes string values with a bool value), but in the line of how 'first'/'last' are worded, if there is a string version for keep=False, shouldn't it rather be keep='none' instead of keep='all' ?

Now that doesn't solve that the 'keep' argument kind of "works backwards" as it are the ones you don't keep which are marked True in duplicated. So for me, it is rather the 'keep' argument name itself that is confusing for duplicated.

[svenharris]

@jorisvandenbossche Yes, sorry I don't think I've articulated this very well, let my try to clarify what I was originally thinking with my pull request. Worth saying this discussion has changed my opinion on what's required slightly.

Thinking behind the original pull request:

• For the drop_duplicates method I feel like keep='first','last',False do an OK job at explaining what's going on.

• For the duplicated method I felt like keep='first','last' kind of made sense and keep=False didn't really. My approach was to add 'all' (I chose 'all' rather than 'none' because the behaviour is to mark all duplicated values in the frame as True) as an alias for False, but really I think I was getting at deprecating False in exchange for 'all'. However I felt this was more complex and would break the reversible behaviour of duplicated and drop_duplicates. So I guess you could say what I did was 'tactical' but didn't really solve the root of the problem.

From the discussion I think these things have become clear:

• keep=False is confusing in both duplicated or drop_duplicates so should consider finding a suitable replacement

• keep keyword in duplicated could maybe be renamed since nothing is actually being dropped or kept (perhaps how or mark or which?)

• Mirrored behaviour in duplicated and drop_duplicates may need to be broken to a degree for the keywords to be meaningful in both contexts

[datapythonista]

+1 here, the current API is very confusing. I don't think the API can be made consistent between duplicated and drop_duplicates, and I think that's were the problem with keep=False from (besides the poor choice of using bool while other values are str).

I think the changes required are:

• Deprecate .drop_duplicates(keep=False) in favor of .drop_duplicates(keep='none')
• Replace keep of duplicated by something else like ignore. So .duplicated(ignore='first') will set all duplicates to True ignoring the first. Same for ignore='last'. And then we can have ignore='none' which would set all duplicates to True (and not ignore any). Not sure if skip or something else could be a better name for the argument, but I think this needs to be the meaning.

Trying to use keep in both doesn't make sense:

This would return all duplicate rows:

>>> df[df.duplicated(keep='all')]

While this would do nothing:

>>> df.drop_duplicates(keep='all')

If using none instead. This would return an empty dataset:

>>> df[df.duplicated(keep='none')]

While this would remove all duplicates and not leave any:

>>> df.drop_duplicates(keep='none')

So, I don't think there is a way that the same parameters can be reused that makes the API clear. And I think keep is confusing in duplicated (keep='first' is not keeping the first, but returning all but the first). keep just makes sense for drop_duplicates.

Happy with this approach @jorisvandenbossche @mroeschke @svenharris ?