Skip to content

Change fname to path in to_parquet #25976

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

Closed

Conversation

ssikdar1
Copy link
Contributor

@ssikdar1 ssikdar1 commented Apr 3, 2019

#23574

Copy link
Member

@WillAyd WillAyd 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. Since this is part of the public API though we can't just straight up change - would have to deprecate fname first

@ssikdar1
Copy link
Contributor Author

ssikdar1 commented Apr 3, 2019

@WillAyd

Ok!

I see other functions in this file use deprecate_kwarg

Looking at decorators:

https://github.com/pandas-dev/pandas/blob/master/pandas/util/_decorators.py#L78

I only see one for deprecating functions and keyword arguments.

Should I just do @deprecate_kwarg(old_arg_name='fname', new_arg_name='path') ?

Or should I just add a deprecated comment in the functiondocstring?

@WillAyd
Copy link
Member

WillAyd commented Apr 3, 2019 via email

@jreback
Copy link
Contributor

jreback commented Apr 3, 2019

this is even more complicated. we have a number of to_* methods. most use fname, but some like to_csv use path_or_buf.

So i think we actually need to decide a common arg and then possibly deprecate these

@WillAyd
Copy link
Member

WillAyd commented Apr 4, 2019

Makes sense - @ssikdar1 I think in light of that this needs a little more discussion, so going to close out this PR because we should agree on approach first.

Do you have a point of view on how to tackle and if so could you post that to the original issue to lead that discussion? That would be the most constructive way to go about this one

@WillAyd WillAyd closed this Apr 4, 2019
@ssikdar1 ssikdar1 deleted the 23574-parquet-fname-to-path branch April 4, 2019 11:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Change Name of fname Parameter to path in Parquet IO Methods
3 participants