Skip to content

Conversation

topper-123
Copy link
Contributor

@topper-123 topper-123 commented May 27, 2020

Follow-up to #27573.

Allows two non-keyword arguments, io and sheet_name. I think sheet_name is quite often (e.g. Interactively) supplied without being a keyword argument and requiring it will just be needlessly annoying.

Also some clean-up in pandas/tests/io/excel.

@topper-123 topper-123 changed the title DEPR: deprecate non keyword arguments to read_excel DEPR: deprecate non keyword arguments in read_excel May 27, 2020
@topper-123 topper-123 marked this pull request as ready for review May 28, 2020 00:27
@@ -33,7 +33,7 @@ def test_read_writer_table():
columns=["Column 1", "Unnamed: 2", "Column 3"],
)

result = pd.read_excel("writertable.odt", "Table1", index_col=0)
result = pd.read_excel("writertable.odt", sheet_name="Table1", index_col=0)
Copy link
Member

Choose a reason for hiding this comment

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

Is sheet_name required to be passed like this or still OK positionally? I assume the latter from allowed_args being 2 but maybe am misreading

+/- 0 on making this required as a keyword argument. It would seem a rather natural positional argument so maybe not worth the churn, but not a strong opinion

Copy link
Contributor Author

@topper-123 topper-123 May 28, 2020

Choose a reason for hiding this comment

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

Is sheet_name required to be passed like this or still OK positionally?

This is just a cleanup, I think it's more readable with keyword arguments.

I think exel files are very well known, so people will easily infer that the second parameter is the sheet name, and making positional argument for sheet_name fail, e.g. in jupyter notebook could maybe be seen as too restrictive. This is not a strong opinion, so if making it a required kwarg is seen as better, I can do that.

@topper-123 topper-123 added IO Excel read_excel, to_excel API - Consistency Internal Consistency of API/Behavior labels May 28, 2020
@topper-123 topper-123 added this to the 1.1 milestone May 28, 2020
@jreback jreback added the Deprecate Functionality to remove in pandas label May 28, 2020
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.

lgtm. can you add this issue to the deprecation removal 2.0 issue; @WillAyd over to you.

@WillAyd WillAyd merged commit 043b609 into pandas-dev:master May 29, 2020
@WillAyd
Copy link
Member

WillAyd commented May 29, 2020

Great PR @topper-123

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API - Consistency Internal Consistency of API/Behavior Deprecate Functionality to remove in pandas IO Excel read_excel, to_excel
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants