BUG/ENH: Fix apply to only call `func` once on the first column/row #34183

alonme · 2020-05-14T21:08:49Z

closes #33879
closes #30815
closes #31620

tests added / passed
passes black pandas
passes git diff upstream/master -u -- "*.py" | flake8 --diff
whatsnew entry

pep8speaks · 2020-05-14T21:08:54Z

Hello @alonme! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2020-06-02 14:00:03 UTC

alonme · 2020-05-14T21:25:33Z

Hope to get this will be merged instead of #33880

There are couple of points i am not sure of, and would like to discuss before wrapping it up completely. they are also pointed in comments to the code

object conversion is done outside of the reduction, i couldn't find a way to do wrap it up in get_result
1.1 If we use current method, i wasn't sure if DataFrame.infer_objects is more fitting than convert_dtypes.
1.2 i hope that by having another method to do that, i would be able to not imoprt Series and DataFrame
I did not provide a full solution for apply_raw, i wanted to make sure the general idea for a solution is valid.
Should i add tests to check that the function is only called once?

pandas/core/apply.py

pandas/tests/frame/test_apply.py

pandas/core/apply.py

pandas/tests/groupby/test_bin_groupby.py

alonme · 2020-05-16T11:30:39Z

as this actually solves also the case for #31695, i can add the doc change (the PR for this issue seems a bit stuck)

pandas/core/apply.py

jreback

you are claiming 3 issues closed. please put up a test for each.

jreback

this needs testing & likely change reduction

pandas/_libs/reduction.pyx

pandas/core/apply.py

alonme · 2020-05-18T19:22:35Z

#31111 wasn't fixed by me.
i was confused as it was solved (?) somewhere between 1.0.3 and master

pandas/tests/frame/test_apply.py

pandas/core/apply.py

alonme · 2020-05-24T18:30:47Z

Hey guys, waiting for your review of how i fixed your comments.
And for a decision regarding the use of unittest.mock

pandas/tests/frame/test_apply.py

jreback

ok looks pretty good. can you add a whatsnew note; this needs an actual section in api breaking (for visibility); you can look back at the groupby apply change and use a similar format here

pandas/_libs/reduction.pyx

pandas/core/apply.py

pandas/tests/frame/test_apply.py

pandas/core/apply.py

jreback · 2020-05-28T13:26:57Z

pandas/core/apply.py

+                    # no exceptions - however reduction was unsuccessful,
+                    # use the computed function result for first element
+                    partial_result = result[0]
+                    if isinstance(partial_result, ABCSeries):


I would actually remove this as its an api change

not averse but that would be a separate change; I think we have an issue below about this

This object inference was added as without it test_replace_method fails.

pandas/core/apply.py

pandas/tests/frame/test_apply.py

alonme · 2020-05-28T14:47:50Z

@jreback thanks for the review!

Important to say that this solution is not complete, i have not solved the case for apply_raw.
I already tried to look at it a bit and couldn't find an elegant way to insert the pre-computed result and not processing it again.

I will try to spend a bit more time on that, However i won't have much time in the near future,
So if you prefer we can merge this and open another issue for raw.

…ults

alonme · 2020-05-31T19:15:54Z

@jreback i fixed/asked a question regarding all issues,

I don't believe i will have time to fix the problem in raw_apply in the next month or so(meaning it will still run twice as it does now for the raw code path).
I believe there should be a simple solution there, its just finding the right way to slice the values passed to apply_along_axis so the first object is not passed. and then add the pre-computed object instead of that first value.

jreback · 2020-05-31T19:17:54Z

@jreback i fixed/asked a question regarding all issues,

I don't believe i will have time to fix the problem in raw_apply in the next month or so(meaning it will still run twice as it does now for the raw code path).
I believe there should be a simple solution there, its just finding the right way to slice the values passed to apply_along_axis so the first object is not passed. and then add the pre-computed object instead of that first value.

can u add a test that shows this case and xfail it; also pls open an issue for this case

alonme · 2020-05-31T21:04:29Z

@jreback
Added the test and opened the issue,

There are 3 open conversations for which i answered reviewers questions and i am waiting for a reviewer to close them/ decide if we need to change code/ open an issue.

jreback

pls update the docs as shown, ping on green.

doc/source/whatsnew/v1.1.0.rst

jreback · 2020-06-02T13:18:37Z

pandas/tests/frame/test_apply.py

+            assert names == list(df.index)
+
+    @pytest.mark.xfail(
+        reason="The 'run once' enhancement for apply_raw not implemented yet."


can you add the issue number you create here

Its listed in comment in the first row of test function,
You want it also in the reason?

alonme · 2020-06-02T14:37:21Z

@jreback green!
Just tell me if you want me to add the issue number also to the xfail reason

jreback · 2020-06-02T21:04:26Z

thanks @alonme very nice & very responsive!

alonme commented May 14, 2020

View reviewed changes

pandas/core/apply.py Outdated Show resolved Hide resolved

pandas/tests/frame/test_apply.py Show resolved Hide resolved

pandas/core/apply.py Outdated Show resolved Hide resolved

alonme commented May 15, 2020

View reviewed changes

pandas/tests/groupby/test_bin_groupby.py Show resolved Hide resolved

alonme changed the title ~~Fix apply to only call func once on the first column/row~~ BUG/ENH: Fix apply to only call func once on the first column/row May 16, 2020

alonme commented May 17, 2020

View reviewed changes

pandas/core/apply.py Outdated Show resolved Hide resolved

alonme force-pushed the gh-33879-fix-df-apply branch 3 times, most recently from 4bf1742 to 8c366be Compare May 18, 2020 13:47

jreback requested changes May 18, 2020

View reviewed changes

pandas/_libs/reduction.pyx Show resolved Hide resolved

pandas/core/apply.py Outdated Show resolved Hide resolved

pandas/core/apply.py Outdated Show resolved Hide resolved

pandas/core/apply.py Outdated Show resolved Hide resolved

jreback added the Reshaping Concat, Merge/Join, Stack/Unstack, Explode label May 18, 2020

alonme mentioned this pull request May 18, 2020

DOC: Removed Notes from DataFrame.applymap #31695

Closed

WillAyd requested changes May 18, 2020

View reviewed changes

pandas/tests/frame/test_apply.py Outdated Show resolved Hide resolved

alonme force-pushed the gh-33879-fix-df-apply branch from ce0e124 to 40646e2 Compare May 19, 2020 21:21

WillAyd requested changes May 20, 2020

View reviewed changes

pandas/core/apply.py Outdated Show resolved Hide resolved

alonme requested review from WillAyd and jreback May 20, 2020 18:20

jreback requested changes May 28, 2020

View reviewed changes

pandas/tests/frame/test_apply.py Show resolved Hide resolved

alonme requested a review from jreback May 28, 2020 06:33

jreback requested changes May 28, 2020

View reviewed changes

jreback reviewed May 28, 2020

View reviewed changes

pandas/tests/frame/test_apply.py Show resolved Hide resolved

alonme added 4 commits May 28, 2020 19:12

Fix apply to only call func once on the first column/row

515de45

Fix groupby reducer tests

b71921b

fix pep8

a8a6fd5

Fix imports for linting

3957956

alonme added 6 commits May 28, 2020 19:12

Add some tests, use ABCseries, remove wrong docs for applymap

4564e9f

Use mock to test call_count

e5bdb20

Remove DataFrame import

1eb9442

Reduction returns success value and partial result is returned in res…

a59934d

…ults

Remove unittest.mock

7ee1faa

Add whatsnew, small CR refactor

fa6299d

alonme force-pushed the gh-33879-fix-df-apply branch from 4196103 to fa6299d Compare May 29, 2020 08:11

alonme added 2 commits May 29, 2020 13:44

Fix doc linting

58cefd6

Fix code blocks in doc

07edc87

alonme requested a review from jreback May 29, 2020 11:59

alonme mentioned this pull request May 31, 2020

BUG: DataFrame.apply(raw=True) applies function twice on first row/column #34506

Closed

3 tasks

Add test for apply_raw with xfail

2cd8ee6

alonme force-pushed the gh-33879-fix-df-apply branch from 86429a5 to 2cd8ee6 Compare May 31, 2020 20:20

jreback requested changes Jun 2, 2020

View reviewed changes

alonme added 2 commits June 2, 2020 16:37

Fix whatsnew python blocks

ec22529

fix indent

55db871

alonme requested a review from jreback June 2, 2020 20:21

jreback added this to the 1.1 milestone Jun 2, 2020

jreback approved these changes Jun 2, 2020

View reviewed changes

jreback merged commit 9a57f45 into pandas-dev:master Jun 2, 2020

alonme mentioned this pull request Jul 24, 2020

CLN: Remove leftover partial-result code from apply #35397

Merged

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

BUG/ENH: Fix apply to only call `func` once on the first column/row #34183

BUG/ENH: Fix apply to only call `func` once on the first column/row #34183

alonme commented May 14, 2020 •

edited

Loading

pep8speaks commented May 14, 2020 •

edited

Loading

alonme commented May 14, 2020

alonme commented May 16, 2020 •

edited

Loading

jreback left a comment

jreback left a comment

alonme commented May 18, 2020

alonme commented May 24, 2020 •

edited

Loading

jreback left a comment

jreback May 28, 2020

jreback May 28, 2020

alonme May 28, 2020

alonme commented May 28, 2020

alonme commented May 31, 2020

jreback commented May 31, 2020

alonme commented May 31, 2020 •

edited

Loading

jreback left a comment

jreback Jun 2, 2020

alonme Jun 2, 2020

alonme commented Jun 2, 2020

jreback commented Jun 2, 2020

BUG/ENH: Fix apply to only call func once on the first column/row #34183

BUG/ENH: Fix apply to only call func once on the first column/row #34183

Conversation

alonme commented May 14, 2020 • edited Loading

pep8speaks commented May 14, 2020 • edited Loading

Comment last updated at 2020-06-02 14:00:03 UTC

alonme commented May 14, 2020

alonme commented May 16, 2020 • edited Loading

jreback left a comment

Choose a reason for hiding this comment

jreback left a comment

Choose a reason for hiding this comment

alonme commented May 18, 2020

alonme commented May 24, 2020 • edited Loading

jreback left a comment

Choose a reason for hiding this comment

jreback May 28, 2020

Choose a reason for hiding this comment

jreback May 28, 2020

Choose a reason for hiding this comment

alonme May 28, 2020

Choose a reason for hiding this comment

alonme commented May 28, 2020

alonme commented May 31, 2020

jreback commented May 31, 2020

alonme commented May 31, 2020 • edited Loading

jreback left a comment

Choose a reason for hiding this comment

jreback Jun 2, 2020

Choose a reason for hiding this comment

alonme Jun 2, 2020

Choose a reason for hiding this comment

alonme commented Jun 2, 2020

jreback commented Jun 2, 2020

BUG/ENH: Fix apply to only call `func` once on the first column/row #34183

BUG/ENH: Fix apply to only call `func` once on the first column/row #34183

alonme commented May 14, 2020 •

edited

Loading

pep8speaks commented May 14, 2020 •

edited

Loading

alonme commented May 16, 2020 •

edited

Loading

alonme commented May 24, 2020 •

edited

Loading

alonme commented May 31, 2020 •

edited

Loading