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

ENH: Add axis parameter to add_prefix and add_suffix #48085

Merged
merged 30 commits into from
Aug 31, 2022

Conversation

dannyi96
Copy link
Contributor

@dannyi96 dannyi96 commented Aug 15, 2022

@dannyi96 dannyi96 marked this pull request as ready for review August 15, 2022 15:19
@dannyi96
Copy link
Contributor Author

dannyi96 commented Aug 19, 2022

Hi @mroeschke, are there any other changes required (or) should we hold back changes in this API for now ?
Accordingly (if required) can update the whatsnew file along with other review suggestions
Thanks in advance. :)

Copy link
Member

@phofl phofl left a comment

Choose a reason for hiding this comment

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

lgtm, but we need a whatsnew

@dannyi96 dannyi96 marked this pull request as draft August 20, 2022 06:57
@dannyi96 dannyi96 marked this pull request as ready for review August 20, 2022 16:06
@pep8speaks
Copy link

pep8speaks commented Aug 23, 2022

Hello @dannyi96! 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 2022-08-30 22:27:53 UTC

Copy link
Member

@phofl phofl left a comment

Choose a reason for hiding this comment

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

cc @mroeschke I think this is ready. If we don't backport, have to change versionadded

@mroeschke
Copy link
Member

mroeschke commented Aug 25, 2022

IMO I think this is more appropriate for 1.6, so I would be in favor of moving the release notes to 1.6.0.rst. I could be convinced otherwise though

@phofl
Copy link
Member

phofl commented Aug 25, 2022

No preference on my side. @dannyi96 could you move whatsnew and change Versionadded?

@dannyi96
Copy link
Contributor Author

Sure thing @mroeschke @phofl. Will move the notes to 1.6

@dannyi96
Copy link
Contributor Author

updated the whatsnew.
( have` added it as part of "Other enhancements" in whatsnew v1.6 file )

pandas/core/generic.py Outdated Show resolved Hide resolved
Copy link
Member

@mroeschke mroeschke left a comment

Choose a reason for hiding this comment

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

One small comment otherwise LGTM. If you could merge in main once more, that would be great.

@dannyi96
Copy link
Contributor Author

One small comment otherwise LGTM. If you could merge in main once more, that would be great.

done 🙂

@mroeschke mroeschke added this to the 1.6 milestone Aug 31, 2022
@mroeschke mroeschke merged commit baf1d2c into pandas-dev:main Aug 31, 2022
@mroeschke
Copy link
Member

Thanks @dannyi96, great work!

@dannyi96 dannyi96 deleted the suffix_prefix_axis branch August 31, 2022 18:00
@mroeschke mroeschke modified the milestones: 1.6, 2.0 Oct 13, 2022
noatamir pushed a commit to noatamir/pandas that referenced this pull request Nov 9, 2022
* Add axis parameter to add_prefix and add_suffix

* Add axis parameter to add_prefix and add_suffix

* Added testcases

* docstring update

* docstring update

* updated whatsnew file

* updated whatsnew file

* updated whatsnew file

* review comments

* address merge conflixts

* address merge conflixts

* updated whatsnew

* review comments

* review comments

* review comments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ENH: Add axis parameter to add_prefix and add_suffix
4 participants