Implement Series.pop #866

itholic · 2019-10-03T06:53:01Z

Resolves #789

>>> s = ks.Series(data=np.arange(3), index=['A', 'B', 'C'])
>>> s
A    0
B    1
C    2
Name: 0, dtype: int64

>>> s.pop('A')
A    0
Name: 0, dtype: int64

>>> s
B    1
C    2
Name: 0, dtype: int64

Also support for MultiIndex

>>> midx = pd.MultiIndex([['lama', 'cow', 'falcon'],
...                       ['speed', 'weight', 'length']],
...                      [[0, 0, 0, 1, 1, 1, 2, 2, 2],
...                       [0, 1, 2, 0, 1, 2, 0, 1, 2]])
>>> s = ks.Series([45, 200, 1.2, 30, 250, 1.5, 320, 1, 0.3],
...               index=midx)
>>> s
lama    speed      45.0
        weight    200.0
        length      1.2
cow     speed      30.0
        weight    250.0
        length      1.5
falcon  speed     320.0
        weight      1.0
        length      0.3
Name: 0, dtype: float64

>>> s.pop('lama')
lama  speed      45.0
      weight    200.0
      length      1.2
Name: 0, dtype: float64

>>> s
cow     speed      30.0
        weight    250.0
        length      1.5
falcon  speed     320.0
        weight      1.0
        length      0.3
Name: 0, dtype: float64

And beyond pandas, we can support for MultiIndex with specifying level.
(But i'm not sure this is necessary.)

>>> midx = pd.MultiIndex([['lama', 'cow', 'falcon'],
...                       ['speed', 'weight', 'length']],
...                      [[0, 0, 0, 1, 1, 1, 2, 2, 2],
...                       [0, 1, 2, 0, 1, 2, 0, 1, 2]])
>>> s = ks.Series([45, 200, 1.2, 30, 250, 1.5, 320, 1, 0.3],
...               index=midx)
>>> s
lama    speed      45.0
        weight    200.0
        length      1.2
cow     speed      30.0
        weight    250.0
        length      1.5
falcon  speed     320.0
        weight      1.0
        length      0.3
Name: 0, dtype: float64

>>> s.pop('speed', level=1)
lama    speed     45.0
cow     speed     30.0
falcon  speed    320.0
Name: 0, dtype: float64

>>> s
lama    weight    200.0
        length      1.2
cow     weight    250.0
        length      1.5
falcon  weight      1.0
        length      0.3
Name: 0, dtype: float64

In pandas, above example raises TypeError like below:

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: pop() got an unexpected keyword argument 'level'

codecov-io · 2019-10-03T18:54:14Z

Codecov Report

Merging #866 into master will increase coverage by 0.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #866      +/-   ##
==========================================
+ Coverage   95.19%   95.21%   +0.01%     
==========================================
  Files          34       34              
  Lines        6810     6835      +25     
==========================================
+ Hits         6483     6508      +25     
  Misses        327      327

Impacted Files	Coverage Δ
databricks/koalas/missing/series.py	`100% <ø> (ø)`	⬆️
databricks/koalas/series.py	`96.53% <100%> (+0.11%)`	⬆️

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 7070bc6...513bd8a. Read the comment docs.

ueshin · 2019-10-03T21:27:33Z

The returned value seems different from pandas'.

>>> s
lama    speed      45.0
        weight    200.0
        length      1.2
cow     speed      30.0
        weight    250.0
        length      1.5
falcon  speed     320.0
        weight      1.0
        length      0.3
dtype: float64
>>> s.pop('lama')
speed      45.0
weight    200.0
length      1.2
dtype: float64
>>> s
cow     speed      30.0
        weight    250.0
        length      1.5
falcon  speed     320.0
        weight      1.0
        length      0.3
dtype: float64

ueshin · 2019-10-03T21:34:23Z

Btw, shall we remove level for now to be consistent with DataFrame's API?

itholic · 2019-10-04T02:04:52Z

@ueshin Thanks for reviewing!! fixed them. :)

itholic · 2019-10-04T02:08:34Z

@ueshin Ah I got it. Then i think maybe our most important purpose is mimic pandas as same as possible, not more, not less, right ? 😃

ueshin · 2019-10-04T02:15:07Z

@itholic Yup, basically that's right. We rarely expand if there is a special reason, though. haha. it depends. 😸

itholic · 2019-10-04T02:18:28Z

@ueshin Got it. Thanks for replying :)

ueshin · 2019-10-04T22:31:17Z

Shall we support this?

>>> s
a  lama    speed      45.0
           weight    200.0
           length      1.2
   cow     speed      30.0
           weight    250.0
           length      1.5
b  falcon  speed     320.0
           weight      1.0
           length      0.3
dtype: float64
>>> s.pop(('a', 'lama'))
speed      45.0
weight    200.0
length      1.2
dtype: float64
>>> s
a  cow     speed      30.0
           weight    250.0
           length      1.5
b  falcon  speed     320.0
           weight      1.0
           length      0.3
dtype: float64

itholic · 2019-10-05T04:59:46Z

@ueshin sure, i'm going to add. Thanks for comment! :)

itholic · 2019-10-05T05:51:12Z

@ueshin , added it 😄

ueshin · 2019-10-07T11:48:34Z

So far, I found the following issues:

For the given s,

>>> s
lama    speed      45.0
        weight    200.0
        length      1.2
cow     speed      30.0
        weight    250.0
        length      1.5
falcon  speed     320.0
        weight      1.0
        length      0.3
Name: 0, dtype: float64

(s + 1).pop('lama') raises AnalysisException:

>>> (s + 1).pop('lama')
Traceback (most recent call last):

...

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Users/ueshin/workspace/databricks-koalas/master/databricks/koalas/series.py", line 3286, in pop
    index_map=self._internal.index_map[len(item):])
  File "/Users/ueshin/workspace/databricks-koalas/master/databricks/koalas/internal.py", line 693, in copy
    column_index=column_index, column_index_names=column_index_names)
  File "/Users/ueshin/workspace/databricks-koalas/master/databricks/koalas/internal.py", line 421, in __init__
    self._data_columns = sdf.select(scol).columns
  File "/Users/ueshin/workspace/databricks-koalas/miniconda/envs/databricks-koalas_3.6/lib/python3.6/site-packages/pyspark/sql/dataframe.py", line 1320, in select
    jdf = self._jdf.select(self._jcols(*cols))
  File "/Users/ueshin/workspace/databricks-koalas/miniconda/envs/databricks-koalas_3.6/lib/python3.6/site-packages/py4j/java_gateway.py", line 1257, in __call__
    answer, self.gateway_client, self.target_id, self.name)
  File "/Users/ueshin/workspace/databricks-koalas/miniconda/envs/databricks-koalas_3.6/lib/python3.6/site-packages/pyspark/sql/utils.py", line 69, in deco
    raise AnalysisException(s.split(': ', 1)[1], stackTrace)
pyspark.sql.utils.AnalysisException: 'Resolved attribute(s) 0#33 missing from __index_level_1__#32,(0 + 1)#51 in operator !Project [(0#33 + cast(1 as double)) AS (0 + 1)#54].;;\n!Project [(0#33 + cast(1 as double)) AS (0 + 1)#54]\n+- Project [__index_level_1__#32, (0 + 1)#51]\n   +- Filter (__index_level_0__#31 = lama)\n      +- Project [__index_level_1__#32, (0#33 + cast(1 as double)) AS (0 + 1)#51, __index_level_0__#31]\n         +- LogicalRDD [__index_level_0__#31, __index_level_1__#32, 0#33], false\n'

s.pop(('lama', 'speed')) raises AssertionError:

>>> s.pop(('lama', 'speed'))
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Users/ueshin/workspace/databricks-koalas/master/databricks/koalas/series.py", line 3470, in __repr__
    pser = self.head(max_display_count + 1)._to_internal_pandas()
  File "/Users/ueshin/workspace/databricks-koalas/master/databricks/koalas/series.py", line 1634, in head
    return _col(self.to_dataframe().head(n))
  File "/Users/ueshin/workspace/databricks-koalas/master/databricks/koalas/series.py", line 1072, in to_frame
    index_map=renamed._internal.index_map,
  File "/Users/ueshin/workspace/databricks-koalas/master/databricks/koalas/internal.py", line 574, in index_map
    assert len(self._index_map) > 0
AssertionError

For the missing key, pandas raises KeyError:

>>> s.to_pandas().pop('horse')
Traceback (most recent call last):
  File "/Users/ueshin/workspace/databricks-koalas/miniconda/envs/databricks-koalas_3.6/lib/python3.6/site-packages/pandas/core/indexes/multi.py", line 1319, in get_value
    return libindex.get_value_at(s, k)
  File "pandas/_libs/index.pyx", line 43, in pandas._libs.index.get_value_at
  File "pandas/_libs/index.pyx", line 48, in pandas._libs.index.get_value_at
  File "pandas/_libs/util.pxd", line 113, in pandas._libs.util.get_value_at
  File "pandas/_libs/util.pxd", line 98, in pandas._libs.util.validate_indexer
TypeError: 'str' object cannot be interpreted as an integer

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Users/ueshin/workspace/databricks-koalas/miniconda/envs/databricks-koalas_3.6/lib/python3.6/site-packages/pandas/core/generic.py", line 809, in pop
    result = self[item]
  File "/Users/ueshin/workspace/databricks-koalas/miniconda/envs/databricks-koalas_3.6/lib/python3.6/site-packages/pandas/core/series.py", line 868, in __getitem__
    result = self.index.get_value(self, key)
  File "/Users/ueshin/workspace/databricks-koalas/miniconda/envs/databricks-koalas_3.6/lib/python3.6/site-packages/pandas/core/indexes/multi.py", line 1327, in get_value
    raise e1
  File "/Users/ueshin/workspace/databricks-koalas/miniconda/envs/databricks-koalas_3.6/lib/python3.6/site-packages/pandas/core/indexes/multi.py", line 1311, in get_value
    return self._engine.get_value(s, k)
  File "pandas/_libs/index.pyx", line 81, in pandas._libs.index.IndexEngine.get_value
  File "pandas/_libs/index.pyx", line 89, in pandas._libs.index.IndexEngine.get_value
  File "pandas/_libs/index.pyx", line 108, in pandas._libs.index.IndexEngine.get_loc
  File "pandas/_libs/index.pyx", line 667, in pandas._libs.index.BaseMultiIndexCodesEngine.get_loc
KeyError: 'horse'

whereas Koalas returns empty Series:

>>> s.pop('horse')
Series([], Name: 0, dtype: float64)

As for 3), the current behavior is reasonable, otherwise we need to count the row number. cc @HyukjinKwon

itholic · 2019-10-07T12:10:57Z

@ueshin Thanks for reviewing!! i'm going to take a look at them.

itholic · 2019-10-07T14:26:20Z

@ueshin Fixed (1), (2) and not yet for (3).
(but fixed (3) & ready for commit in my local code)

databricks/koalas/series.py

itholic · 2019-10-14T14:33:39Z

hmm.. i don't understand why travis failed only in python3.6

databricks/koalas/series.py

HyukjinKwon · 2019-11-06T02:42:25Z

@itholic can you rebase this please?

itholic · 2019-11-06T05:24:24Z

@HyukjinKwon Sure, i just rebased them :)

databricks/koalas/series.py

HyukjinKwon

Looks good otherwise

softagram-bot · 2019-11-22T05:35:14Z

Softagram Impact Report for pull/866 (head commit: `513bd8a`)

⭐ Change Overview

(Open in Softagram Desktop for full details)

📄 Full report

Permalink: Full report for pull/866

Impact Report explained. Give feedback on this report to [email protected]

itholic added 4 commits October 3, 2019 15:44

Implement Series.pop

ec8bd32

Add to doc

231b1e8

Merge branch 'master' of https://github.com/databricks/koalas into s_pop

e7511e9

Remove duplicated docstring

be0d1f7

itholic added 2 commits October 4, 2019 11:01

Fix to exactly same with pandas & remove parameter 'level'

32ec69c

Fix to exactly same with pandas & remove parameter 'level'

b4d8336

itholic added 2 commits October 4, 2019 12:34

Remove unused import

f44e916

Merge branch 'master' of https://github.com/databricks/koalas into s_pop

4baff91

itholic added 3 commits October 5, 2019 14:51

Support multiindex with several indexes & add some tests

0c6af6c

Change code order for proper exception handling

e32140c

Fix docstring for python3.5 compatiblity

28f7cae

Fix some bugs

0dd788a

Add test case

dc75c45

ueshin reviewed Oct 8, 2019

View reviewed changes

databricks/koalas/series.py Outdated Show resolved Hide resolved

databricks/koalas/series.py Outdated Show resolved Hide resolved

itholic added 3 commits October 8, 2019 21:42

Consolidate logic between single/multi index

3f8ec59

Resolve conflicts

c5c0c3f

Dont't invoke count()

407c8bc

ueshin reviewed Oct 8, 2019

View reviewed changes

databricks/koalas/series.py Outdated Show resolved Hide resolved

databricks/koalas/series.py Outdated Show resolved Hide resolved

itholic and others added 8 commits October 10, 2019 13:20

Fix logic when result has only one column

fa3ba3b

Resolve lint fail

34aefe8

add a test case & fix code related with it

64b3674

Resolve conflicts

3a70a12

remove unused import

77f79e1

Merge branch 'master' of https://github.com/databricks/koalas into s_pop

f1bfd1d

koalas -> ks

7cafab5

Merge branch 's_pop' of https://github.com/itholic/koalas into s_pop

4551a8d

ueshin reviewed Oct 14, 2019

View reviewed changes

databricks/koalas/series.py Outdated Show resolved Hide resolved

databricks/koalas/series.py Outdated Show resolved Hide resolved

Reuse existing logic

ccf1610

itholic added 3 commits October 15, 2019 14:37

Merge branch 'master' of https://github.com/databricks/koalas into s_pop

e2dfee1

_index_map -> _internal._index_map

4d5ad47

Add test case

e7e5f74

ueshin reviewed Oct 18, 2019

View reviewed changes

databricks/koalas/series.py Show resolved Hide resolved

ueshin reviewed Oct 18, 2019

View reviewed changes

databricks/koalas/series.py Show resolved Hide resolved

itholic added 2 commits October 20, 2019 10:59

check whether key length exceeds index depth or not

d9318a8

Resolve conflicts

f4e6f1d

Resolve conflicts

2189355

HyukjinKwon reviewed Nov 22, 2019

View reviewed changes

databricks/koalas/series.py Outdated Show resolved Hide resolved

HyukjinKwon approved these changes Nov 22, 2019

View reviewed changes

itholic added 2 commits November 22, 2019 14:30

Merge remote-tracking branch 'upstream/master' into s_pop

a562456

fix

513bd8a

HyukjinKwon merged commit dad31db into databricks:master Nov 22, 2019

itholic deleted the s_pop branch December 2, 2019 15:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement Series.pop #866

Implement Series.pop #866

itholic commented Oct 3, 2019

codecov-io commented Oct 3, 2019 •

edited

Loading

ueshin commented Oct 3, 2019

ueshin commented Oct 3, 2019

itholic commented Oct 4, 2019

itholic commented Oct 4, 2019

ueshin commented Oct 4, 2019 •

edited

Loading

itholic commented Oct 4, 2019

ueshin commented Oct 4, 2019

itholic commented Oct 5, 2019 •

edited

Loading

itholic commented Oct 5, 2019

ueshin commented Oct 7, 2019

itholic commented Oct 7, 2019

itholic commented Oct 7, 2019 •

edited

Loading

itholic commented Oct 14, 2019

HyukjinKwon commented Nov 6, 2019

itholic commented Nov 6, 2019

HyukjinKwon left a comment

softagram-bot commented Nov 22, 2019

Implement Series.pop #866

Implement Series.pop #866

Conversation

itholic commented Oct 3, 2019

codecov-io commented Oct 3, 2019 • edited Loading

Codecov Report

ueshin commented Oct 3, 2019

ueshin commented Oct 3, 2019

itholic commented Oct 4, 2019

itholic commented Oct 4, 2019

ueshin commented Oct 4, 2019 • edited Loading

itholic commented Oct 4, 2019

ueshin commented Oct 4, 2019

itholic commented Oct 5, 2019 • edited Loading

itholic commented Oct 5, 2019

ueshin commented Oct 7, 2019

itholic commented Oct 7, 2019

itholic commented Oct 7, 2019 • edited Loading

itholic commented Oct 14, 2019

HyukjinKwon commented Nov 6, 2019

itholic commented Nov 6, 2019

HyukjinKwon left a comment

Choose a reason for hiding this comment

softagram-bot commented Nov 22, 2019

Softagram Impact Report for pull/866 (head commit: 513bd8a)

⭐ Change Overview

📄 Full report

codecov-io commented Oct 3, 2019 •

edited

Loading

ueshin commented Oct 4, 2019 •

edited

Loading

itholic commented Oct 5, 2019 •

edited

Loading

itholic commented Oct 7, 2019 •

edited

Loading

Softagram Impact Report for pull/866 (head commit: `513bd8a`)