Skip to content

CLN: some cleanups in Series.apply & related #52786

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

Merged
merged 1 commit into from
Apr 19, 2023

Conversation

topper-123
Copy link
Contributor

@topper-123 topper-123 commented Apr 19, 2023

Some cleanups:

  • Move the convert_dtype warning from Series.apply to SeriesApply
  • Add *, to force use use of keywords
  • Set the default for convert_dtype in SeriesApply to no_default
  • Drop setting convert_dtype internally in Series.agg & Series.transform (these are noops, i.e. have no effect)

@topper-123 topper-123 changed the title CLN: some cleanups in Series.qpply & related CLN: some cleanups in Series.apply & related Apr 19, 2023
@mroeschke mroeschke added Clean Apply Apply, Aggregate, Transform, Map labels Apr 19, 2023
@mroeschke mroeschke added this to the 2.1 milestone Apr 19, 2023
@mroeschke mroeschke merged commit 6e91a22 into pandas-dev:main Apr 19, 2023
@mroeschke
Copy link
Member

Thanks @topper-123

@topper-123 topper-123 deleted the cln_apply branch April 19, 2023 22:26
@phofl
Copy link
Member

phofl commented Apr 25, 2023

it looks like this caused a performance regression https://asv-runner.github.io/asv-collection/pandas/#join_merge.ConcatDataFrames.time_c_ordered?p-axis=0

could you take a closer look?

@topper-123
Copy link
Contributor Author

I've bisected this and the regression is actually caused by #52685.

@topper-123 topper-123 mentioned this pull request Apr 26, 2023
5 tasks
@phofl
Copy link
Member

phofl commented Apr 26, 2023

thx, this pr looked weird, but didn't have time to investigate more. The other makes more sense

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Apply Apply, Aggregate, Transform, Map Clean
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants