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.

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

Added FrozenList subtraction #15506

Closed
wants to merge 22 commits into from

Conversation

Projects
None yet
6 participants
@bthayer2365
Copy link
Contributor

commented Feb 25, 2017

  • closes #15475
  • tests added / passed
  • passes git diff upstream/master | flake8 --diff
  • whatsnew entry

bthayer2365 added some commits Feb 25, 2017

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

This comment has been minimized.

Copy link
@jreback

jreback Feb 25, 2017

Contributor

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`)

This comment has been minimized.

Copy link
@jreback

jreback Feb 25, 2017

Contributor

this is called difference

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

@codecov-io

This comment has been minimized.

Copy link

commented Feb 25, 2017

Codecov Report

Merging #15506 into master will decrease coverage by -0.03%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master   #15506      +/-   ##
==========================================
- Coverage   91.06%   91.04%   -0.03%     
==========================================
  Files         136      136              
  Lines       49099    49182      +83     
==========================================
+ Hits        44714    44779      +65     
- Misses       4385     4403      +18
Impacted Files Coverage Δ
pandas/indexes/frozen.py 93.24% <100%> (-0.31%)
pandas/io/gbq.py 40% <0%> (-46.67%)
pandas/util/decorators.py 67.27% <0%> (-1.96%)
pandas/tools/merge.py 91.78% <0%> (-0.35%)
pandas/util/testing.py 81.87% <0%> (-0.19%)
pandas/core/frame.py 97.74% <0%> (-0.1%)
pandas/core/algorithms.py 94.48% <0%> (ø)
pandas/core/categorical.py 96.92% <0%> (+0.01%)
pandas/formats/style.py 96.8% <0%> (+0.63%)

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 2340fb8...428a1b3. Read the comment docs.

@@ -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

This comment has been minimized.

Copy link
@jreback

jreback Feb 25, 2017

Contributor

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):

This comment has been minimized.

Copy link
@jreback

jreback Feb 25, 2017

Contributor

can you rename this to make consistent

@cpcloud cpcloud self-assigned this Feb 25, 2017

@cpcloud cpcloud added this to the 0.20.0 milestone Feb 25, 2017

@jorisvandenbossche

This comment has been minimized.

Copy link
Member

commented Feb 26, 2017

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)

@jreback

This comment has been minimized.

Copy link
Contributor

commented Feb 26, 2017

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.

@bthayer2365

This comment has been minimized.

Copy link
Contributor Author

commented Feb 26, 2017

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

@jreback

This comment has been minimized.

Copy link
Contributor

commented Feb 26, 2017

yes; u will have to deprecate the add though

@bthayer2365

This comment has been minimized.

Copy link
Contributor Author

commented Feb 26, 2017

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

@jreback

This comment has been minimized.

Copy link
Contributor

commented Feb 26, 2017

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

bthayer2365 added some commits Feb 26, 2017


.. ipython:: python
df.set_index(['A', 'B'])

This comment has been minimized.

Copy link
@jreback

jreback Feb 27, 2017

Contributor

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`)

This comment has been minimized.

Copy link
@jreback

jreback Feb 27, 2017

Contributor

.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):

This comment has been minimized.

Copy link
@jreback

jreback Feb 27, 2017

Contributor
return self.union(other)
@@ -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):

This comment has been minimized.

Copy link
@jreback

jreback Feb 27, 2017

Contributor

can you add a doc-string

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

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

This comment has been minimized.

Copy link
@jreback

jreback Feb 27, 2017

Contributor

same here

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

def test_add(self):

This comment has been minimized.

Copy link
@jreback

jreback Feb 27, 2017

Contributor

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))

This comment has been minimized.

Copy link
@jreback

jreback Feb 27, 2017

Contributor

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

@bthayer2365 bthayer2365 force-pushed the bthayer2365:frozen-index branch from 53789b3 to 6a2b48d Mar 1, 2017

@bthayer2365

This comment has been minimized.

Copy link
Contributor Author

commented Mar 2, 2017

Anything else? All tests passed 👍

@jreback jreback closed this in 37fe2c4 Mar 2, 2017

@jreback

This comment has been minimized.

Copy link
Contributor

commented Mar 2, 2017

thanks @bthayer2365

@jsexauer jsexauer referenced this pull request Mar 2, 2017

Open

DEPR: deprecations from prior versions #6581

0 of 94 tasks complete

jreback added a commit to jreback/pandas that referenced this pull request Mar 2, 2017

mcocdawc added a commit to mcocdawc/pandas that referenced this pull request Mar 2, 2017

ENH: Added FrozenList difference setop
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 added a commit to mcocdawc/pandas that referenced this pull request Mar 2, 2017

@cpcloud

This comment has been minimized.

Copy link
Member

commented Mar 2, 2017

@bthayer2365 Great job! Thanks for your contribution.

@bthayer2365

This comment has been minimized.

Copy link
Contributor Author

commented Mar 3, 2017

Glad I could help!

jreback added a commit that referenced this pull request Mar 4, 2017

Revert FrozenList changes (doc build slowdown, #15559)
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 added a commit to mcocdawc/pandas that referenced this pull request Mar 6, 2017

ENH: Added FrozenList difference setop
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

jreback added a commit to mcocdawc/pandas that referenced this pull request Mar 6, 2017

AnkurDedania added a commit to AnkurDedania/pandas that referenced this pull request Mar 21, 2017

ENH: Added FrozenList difference setop
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

AnkurDedania added a commit to AnkurDedania/pandas that referenced this pull request Mar 21, 2017

AnkurDedania added a commit to AnkurDedania/pandas that referenced this pull request Mar 21, 2017

Revert FrozenList changes (doc build slowdown, pandas-dev#15559)
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 added a commit that referenced this pull request Nov 3, 2018

JustinZhengBC added a commit to JustinZhengBC/pandas that referenced this pull request Nov 14, 2018

brute4s99 added a commit to brute4s99/pandas that referenced this pull request Nov 19, 2018

Pingviinituutti added a commit to Pingviinituutti/pandas that referenced this pull request Feb 28, 2019

Pingviinituutti added a commit to Pingviinituutti/pandas that referenced this pull request Feb 28, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.