Skip to content

CLN: Consistency of arguments in to_html and to_string #23612

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
datapythonista opened this issue Nov 10, 2018 · 3 comments · Fixed by #23614
Closed

CLN: Consistency of arguments in to_html and to_string #23612

datapythonista opened this issue Nov 10, 2018 · 3 comments · Fixed by #23614
Labels
Clean good first issue IO HTML read_html, to_html, Styler.apply, Styler.applymap Needs Discussion Requires discussion from core team before further action
Milestone

Comments

@datapythonista
Copy link
Member

datapythonista commented Nov 10, 2018

The methods to_html and to_string share most of the parameters, together with DataFrameFormatter, but there are some differences in the order, and in some parameters that probably should be the same.

Those are the common parameters:

buf=None,
columns=None,
col_space=None,
header=True,
index=True,
na_rep='NaN',
formatters=None,
float_format=None,
sparsify=None,
index_names=True,
justify=None,
max_rows=None,
max_cols=None,
show_dimensions=False

But after formatters they are in different order, which adds complexity (for example in reusing docstrings, or in switching from one function to another when using positional arguments).

Then, those are the parameters that are not shared among the 3:

DataFrameFormatter
--
frame,  # (positional, at the beginning)
line_width=None,
decimal='.',
table_id=None,

to_string
--
line_width=None,

to_html
--
decimal='.',
table_id=None
bold_rows=True,
classes=None,
escape=True,
notebook=False,
border=None,

While that looks mostly ok, it feels like probably to_string should also have a decimal argument (and not sure what DataFrameFormatter should have.

What I propose is:

  • Reorder the arguments so the common ones are located first, and in the same order across all methods.
  • Add decimal to to_string, and check if any other addition (or deletion) makes sense.
  • Add a note in whatsnew, but don't do any deprecation (which would make things quite tricky in our code). Hopefully no users are using positional arguments for the last arguments in a method with around 15.
  • Make sure the docstrings have the parameters in the right order.
  • Looks like in pandas/io/formats/format.py there is a related docstring_to_string variable that is not being used, so we should remove it.

@jreback happy to make these changes?

CC @thoo

@datapythonista datapythonista added IO HTML read_html, to_html, Styler.apply, Styler.applymap Effort Low Clean Needs Discussion Requires discussion from core team before further action good first issue labels Nov 10, 2018
@jreback
Copy link
Contributor

jreback commented Nov 10, 2018

as i said ok with changing to be consistent
ideally don’t move the first few args (which are the only realistic positonally passed ones)
otherwise ok -

@datapythonista
Copy link
Member Author

Thanks. I think the first change would be in the 8th argument.

@thoo, will you work on this?

@thoo
Copy link
Contributor

thoo commented Nov 10, 2018

Sure.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Clean good first issue IO HTML read_html, to_html, Styler.apply, Styler.applymap Needs Discussion Requires discussion from core team before further action
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants