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.

Sign up for GitHub

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

Jump to bottom

CLN: Removed the kwds param in to_csv #13804

Merged
merged 1 commit into from
Jul 29, 2016
Merged

CLN: Removed the kwds param in to_csv #13804

merged 1 commit into from
Jul 29, 2016

Conversation

gfyoung
Copy link
Member

@gfyoung gfyoung commented Jul 26, 2016

We aren't using it anymore with the removal of parameters like engine. Closes #8206.

@jorisvandenbossche jorisvandenbossche added the IO CSV read_csv, to_csv label Jul 26, 2016
@jorisvandenbossche jorisvandenbossche added this to the 0.19.0 milestone Jul 26, 2016
@gfyoung
Copy link
Member Author

gfyoung commented Jul 27, 2016 •
edited
Loading

Boo...tests and the Series object were abusing the **kwds parameter. Though slightly awkward since I was the one who removed the engine parameter in to_csv and forgot about these tests. 😢

@codecov-io
Copy link

codecov-io commented Jul 27, 2016 •
edited
Loading

Current coverage is 85.23% (diff: 100%)

No coverage report found for master at 2c55f28.

Powered by Codecov. Last update 2c55f28...f096812

jorisvandenbossche
jorisvandenbossche reviewed Jul 27, 2016
View reviewed changes
doc/source/whatsnew/v0.19.0.txt
@@ -330,6 +330,7 @@ API changes
~~~~~~~~~~~


- ``Series.to_csv`` has dropped the ``nanRep`` parameter in favor of ``na_rep`` (:issue:`13804`)
Copy link
Member

Choose a reason for hiding this comment

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

It had not really been dropped 'in favor of', I think? As it did actually not have any effect?

Copy link
Contributor

Choose a reason for hiding this comment

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

this was not deprecated I think (but let's still remove)

Copy link
Contributor

Choose a reason for hiding this comment

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

sorry, just saw @jorisvandenbossche comment. yep let's remove.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep, sounds good. Done.

@jorisvandenbossche
Copy link
Member

jorisvandenbossche commented Jul 27, 2016 •
edited
Loading

So we did actually already remove nanRep in 0.14 (#6813, it was deprecated before), but it seems that it was forgotten to remove it in Series.to_csv as well. And also it is still listed in the docstring of DataFrame.to_csv. Can you remove that as well?

jreback
jreback reviewed Jul 28, 2016
View reviewed changes
doc/source/whatsnew/v0.19.0.txt
@@ -332,6 +332,7 @@ API changes
~~~~~~~~~~~


- ``Series.to_csv`` has dropped the ``nanRep`` parameter (:issue:`13804`)
Copy link
Contributor

@jreback jreback Jul 28, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

put here that it is replaced by nan_rep. I think you can put this in the deprecated removals section as it WAS deprecated.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

@gfyoung
Copy link
Member Author

gfyoung commented Jul 29, 2016 •
edited
Loading

@jreback , @jorisvandenbossche : made the requested doc changes (hence the [ci skip]). Ready to merge if there are no other concerns.

@jorisvandenbossche jorisvandenbossche merged commit 748787d into pandas-dev:master Jul 29, 2016
@jorisvandenbossche
Copy link
Member

@gfyoung Thanks!

@gfyoung gfyoung deleted the to-csv-rm-kwds branch July 29, 2016 07:58
@zrisher zrisher mentioned this pull request Dec 8, 2016
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
IO CSV read_csv, to_csv
Projects
None yet
Milestone
0.19.0
Development

Successfully merging this pull request may close these issues.
4 participants
@gfyoung @codecov-io @jorisvandenbossche @jreback