Skip to content

Conversation

@r0cketr1kky
Copy link
Contributor

Documentation screenshot:
Screenshot from 2020-02-05 17-08-33

Copy link
Contributor

@TomAugspurger TomAugspurger left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! There's a linting issue at https://github.com/pandas-dev/pandas/pull/31695/checks?check_run_id=427378996#step:10:25.

I think you may need o remove a newline somewhere. Not entirely sure where though.

@r0cketr1kky
Copy link
Contributor Author

Please review this @TomAugspurger

@MarcoGorelli
Copy link
Member

Thanks @r0cketr1kky

Looks like you've checked in some unwanted files (e.g. doc/make.spec) - could you please unstage them?

Comment on lines 6993 to 6985
Note that a vectorized version of `func` often exists, which will
be much faster. You could square each number elementwise.
You could square each number elementwise.
Copy link
Member

Choose a reason for hiding this comment

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

This part doesn't need changing, the vectorised version is still faster:

In [28]: s = pd.Series(np.random.randn(10000))                                                                                                               

In [29]: %timeit s.apply(lambda x: x**2)                                                                                                                     
6.39 ms ± 5.73 µs per loop (mean ± std. dev. of 7 runs, 100 loops each)

In [30]: %timeit s**2                                                                                                                                        
243 µs ± 4.38 µs per loop (mean ± std. dev. of 7 runs, 1000 loops each)

In [31]: pd.testing.assert_series_equal(s.apply(lambda x: x**2), s**2) 

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh okay. Thanks a lot. Included it.

@pep8speaks
Copy link

pep8speaks commented Feb 5, 2020

Hello @r0cketr1kky! 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-03-31 13:43:31 UTC

@r0cketr1kky
Copy link
Contributor Author

Please review this!

Copy link
Contributor

@jreback jreback left a comment

Choose a reason for hiding this comment

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

if you can only have the 1 change in this PR

"""

_num_doc_mad = """
%(desc)s
Copy link
Contributor

Choose a reason for hiding this comment

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

this is an unrelated change, can you revert

@jreback jreback added the Docs label Feb 9, 2020
Notes
-----
In the current implementation applymap calls `func` twice on the
Copy link
Member

Choose a reason for hiding this comment

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

is this no longer accurate? we regularly get questions about the user-defined functions being called in groupby.apply/agg to determine fast vs slow path

Copy link
Member

Choose a reason for hiding this comment

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

In #28854 this was removed from DataFrame.apply, and there are comments on the original issue (#28827) saying that the implementation has changed recently, and DataFrame.applymap directly calls DataFrame.apply

@datapythonista
Copy link
Member

This seems stale. @r0cketr1kky if you want to finish this (mainly remove the unrelated changes), please open a PR or ping us here to reopen. Thanks!

@r0cketr1kky
Copy link
Contributor Author

Sorry for being late! I just noticed this @datapythonista ! Can you reopen this? I'll revert back with the changes.

@jreback
Copy link
Contributor

jreback commented Apr 10, 2020

@r0cketr1kky can you merge master and update

be much faster. You could square each number elementwise.
Note that a vectorized version of `func` often exists,
which will be much faster.
You could square each number elementwise.
Copy link
Member

Choose a reason for hiding this comment

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

can you revert this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, for being late! I'm having a very busy semester. I'll do this asap and get back!

@alonme
Copy link
Contributor

alonme commented May 18, 2020

It doesn't look to me like this issue is actually solved.
I hope to get it solved in #34183

@MarcoGorelli
Copy link
Member

Closing as #34183 was merged and closed the issue - congrats @alonme for a really impressive contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Does #28827 also apply to applymap?

8 participants