Added FrozenList subtraction by benthayer · Pull Request #15506 · pandas-dev/pandas (original) (raw)

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.

By clicking “Sign up for GitHub”, you agree to our terms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation30 Commits22 Checks0 Files changed

Conversation

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})

closes Groupby all levels except the specified ones #15475
tests added / passed
passes git diff upstream/master | flake8 --diff
whatsnew entry

def __sub__(self, other):
other = set(other)
temp = [x for x in self if x not in other]
return self.__class__(temp)

Choose a reason for hiding this comment

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

add isub as well (and a test)

@@ -26,6 +26,8 @@ Check the :ref:`API Changes <whatsnew_0200.api_breaking>` and :ref:`deprecations
New features
~~~~~~~~~~~~

- Added subtraction from FrozenLists (:issue:`15475`)

Choose a reason for hiding this comment

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

this is called difference

i think might be nice to add an example to th docs as well

@@ -126,6 +126,14 @@ We could naturally group by either the ``A`` or ``B`` columns or both:
grouped = df.groupby('A')
grouped = df.groupby(['A', 'B'])

If we also have a MultiIndex on columns ``A`` and ``B``, we can group by all
but the specified columns

Choose a reason for hiding this comment

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

add a versionadded tag here

result = FrozenList([1, 2, 3, 2]) - [2]
expected = FrozenList([1, 3])
self.check_result(result, expected)

def test_inplace(self):

Choose a reason for hiding this comment

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

can you rename this to make consistent

I want to raise some concern regarding adding this feature (without wanting to dismiss your effort @bthayer2365 !). I think we should think about this some more.

Adding this would introduce a discrepancy between index level names and column names, while we actually try to lessen the gap (eg by being able to specify an index level name in places where you can specify a column name). This is actually a feature we removed from column names, in favor of the difference method.
So that is maybe also an alternative? Putting this in a method instead of overloading subtraction? (although I certainly agree what we try to obtain here is useful, and as a method it is more verbose)

no conceptual problem with making these real setops, e.g. .union, .difference as @jorisvandenbossche suggests. Publicly these are only used for names of levels anyhow.

Would you like me to change __add__ to union as well?

yes; u will have to deprecate the add though

To do that, do I just decorate it with @deprecated, or is there more?

that should work (and add in whatsnew as well)
iadd too


.. ipython:: python

df.set_index(['A', 'B'])

Choose a reason for hiding this comment

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

this won't work as it returns a new object, instead something like

df2 = df.set_index(['A', 'B'])
grouped = df2.groupby(level=....)

@@ -26,6 +26,8 @@ Check the :ref:`API Changes <whatsnew_0200.api_breaking>` and :ref:`deprecations
New features
~~~~~~~~~~~~

- Added difference from FrozenLists (:issue:`15475`)

Choose a reason for hiding this comment

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

.difference() method for FrozenList

@@ -25,6 +27,7 @@ class FrozenList(PandasObject, list):
# typechecks

def __add__(self, other):
warnings.warn("__add__ is deprecated, use union(...)", FutureWarning)
if isinstance(other, tuple):

Choose a reason for hiding this comment

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

@@ -80,6 +83,16 @@ def __repr__(self):
__setitem__ = __setslice__ = __delitem__ = __delslice__ = _disabled
pop = append = extend = remove = sort = insert = _disabled

def union(self, other):
if isinstance(other, tuple):

Choose a reason for hiding this comment

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

can you add a doc-string

return self.__class__(super(FrozenList, self).__add__(other))

def difference(self, other):
other = set(other)

Choose a reason for hiding this comment

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

same here

@@ -14,21 +14,20 @@ def setUp(self):
self.container = FrozenList(self.lst)
self.klass = FrozenList

def test_add(self):

Choose a reason for hiding this comment

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

can you add a test for assert_produces_warning(FutureWarning) for the deprecation of __add__

def union(self, other):
if isinstance(other, tuple):
other = list(other)
return self.__class__(super(FrozenList, self).__add__(other))

Choose a reason for hiding this comment

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

I think __iadd__ is also defined? if so, deprecate that as well (no replacement, just a deprecation)

Anything else? All tests passed 👍

jreback added a commit to jreback/pandas that referenced this pull request

Mar 2, 2017

mcocdawc pushed a commit to mcocdawc/pandas that referenced this pull request

Mar 2, 2017

closes pandas-dev#15475

Author: Ben Thayer benthayer2365@gmail.com Author: bthayer2365 bthayer2365@users.noreply.github.com

Closes pandas-dev#15506 from bthayer2365/frozen-index and squashes the following commits:

428a1b3 [Ben Thayer] Added iadd test, fixed whatsnew 84ba405 [Ben Thayer] Merge branch 'master' of github.com:pandas-dev/pandas into frozen-index 8dbde1e [Ben Thayer] Rebased to upstream/master 6f6c140 [Ben Thayer] Added docstrings, depricated iadd, changed add to use self.union() 66b3b91 [Ben Thayer] Fixed issue number 3d6cee5 [Ben Thayer] Depricated add in favor of union ccd75c7 [Ben Thayer] Changed sub to difference cd7de26 [Ben Thayer] Added versionadded tag in docs and renamed test_inplace to test_inplace_add for consistency 0ea8d21 [Ben Thayer] Added isub and groupby example to docs 79dd958 [Ben Thayer] Updated whatsnew to reflect changes 0fc7e19 [Ben Thayer] Removed whitespace 73564ab [Ben Thayer] Added FrozenList subtraction fee7a7d [bthayer2365] Merge branch 'master' into frozen-index 6a2b48d [Ben Thayer] Added docstrings, depricated iadd, changed add to use self.union() 2ab85cb [Ben Thayer] Fixed issue number cb95089 [Ben Thayer] Depricated add in favor of union 2e43849 [Ben Thayer] Changed sub to difference fdcfbbb [Ben Thayer] Added versionadded tag in docs and renamed test_inplace to test_inplace_add for consistency 2fad2f7 [Ben Thayer] Added isub and groupby example to docs cd73faa [Ben Thayer] Updated whatsnew to reflect changes f6381a8 [Ben Thayer] Removed whitespace ada7cda [Ben Thayer] Added FrozenList subtraction

mcocdawc pushed a commit to mcocdawc/pandas that referenced this pull request

Mar 2, 2017

@bthayer2365 Great job! Thanks for your contribution.

jreback pushed a commit that referenced this pull request

Mar 4, 2017

See #15559. This temporarily reverts #15506, to see if this fixes the doc build slowdown.

Author: Joris Van den Bossche jorisvandenbossche@gmail.com

Closes #15566 from jorisvandenbossche/revert and squashes the following commits:

befd858 [Joris Van den Bossche] Revert "ENH: Added FrozenList difference setop" 527ded9 [Joris Van den Bossche] Revert "TST: remove deprecated usages of FrozenList.add from test code"

jreback pushed a commit to mcocdawc/pandas that referenced this pull request

Mar 6, 2017

closes pandas-dev#15475

Author: Ben Thayer benthayer2365@gmail.com Author: bthayer2365 bthayer2365@users.noreply.github.com

Closes pandas-dev#15506 from bthayer2365/frozen-index and squashes the following commits:

jreback added a commit to mcocdawc/pandas that referenced this pull request

Mar 6, 2017

AnkurDedania pushed a commit to AnkurDedania/pandas that referenced this pull request

Mar 21, 2017

closes pandas-dev#15475

Author: Ben Thayer benthayer2365@gmail.com Author: bthayer2365 bthayer2365@users.noreply.github.com

Closes pandas-dev#15506 from bthayer2365/frozen-index and squashes the following commits:

AnkurDedania pushed a commit to AnkurDedania/pandas that referenced this pull request

Mar 21, 2017

AnkurDedania pushed a commit to AnkurDedania/pandas that referenced this pull request

Mar 21, 2017

See pandas-dev#15559. This temporarily reverts pandas-dev#15506, to see if this fixes the doc build slowdown.

Author: Joris Van den Bossche jorisvandenbossche@gmail.com

Closes pandas-dev#15566 from jorisvandenbossche/revert and squashes the following commits:

befd858 [Joris Van den Bossche] Revert "ENH: Added FrozenList difference setop" 527ded9 [Joris Van den Bossche] Revert "TST: remove deprecated usages of FrozenList.add from test code"

gfyoung added a commit to forking-repos/pandas that referenced this pull request

Oct 28, 2018

gfyoung added a commit to forking-repos/pandas that referenced this pull request

Nov 1, 2018

gfyoung added a commit to forking-repos/pandas that referenced this pull request

Nov 1, 2018

jreback pushed a commit that referenced this pull request

Nov 3, 2018

JustinZhengBC pushed a commit to JustinZhengBC/pandas that referenced this pull request

Nov 14, 2018

tm9k1 pushed a commit to tm9k1/pandas that referenced this pull request

Nov 19, 2018

Pingviinituutti pushed a commit to Pingviinituutti/pandas that referenced this pull request

Feb 28, 2019

Pingviinituutti pushed a commit to Pingviinituutti/pandas that referenced this pull request

Feb 28, 2019