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

REF: pandas/io/formats/format.py #36407

Closed
ivanovmg opened this issue Sep 16, 2020 · 3 comments
Closed

REF: pandas/io/formats/format.py #36407

ivanovmg opened this issue Sep 16, 2020 · 3 comments
Assignees
Labels
Output-Formatting __repr__ of pandas objects, to_string Refactor Internal refactoring of code

Comments

@ivanovmg
Copy link
Member

Is your feature request related to a problem?

class DataFrameFormatter seems mostly to target console formatting, while it plays a role of the mediator for to_latex and to_html conversions.
It is noticeable, that some kwargs, like table_id, render_links are pertaining to html only. Meanwhile dataframe truncation (present in init) is not applicable to latex formatting.
Probably needs refactoring.

Describe the solution you'd like

I suggest creating TableFormatterAbstract class.
Inherit from it: ConsoleFormatter, HTMLFormatter and LatexFormatter.
Put all the required input parameters into the corresponding init of each class (would not need to provide unnecessary kwargs).
Generic functionality like dataframe truncation can be shared via a mixin class TruncateMixin.
Drop DataFrameFormatter and instead of it call proper XFormatter where required.

API breaking implications

Should not affect public API, as will deal only with the internal implementation.

Describe alternatives you've considered

Separate ConsoleFormatter, similarly to HTMLFormatter and LatexFormatter and call it from DataFrameFormatter.to_string method.
This, however will leave DataFrameFormatter with only three public methods to_string, to_html and to_latex.
The problem here is that we still carry unnecessary kwargs, which are not applicable to a particular formatting.

Additional context

I would be interested to work on this. Would the team be interested in such refactoring?

@ivanovmg ivanovmg added Enhancement Needs Triage Issue that has not been reviewed by a pandas team member Refactor Internal refactoring of code and removed Enhancement labels Sep 16, 2020
@jreback
Copy link
Contributor

jreback commented Sep 16, 2020

sure glad to take internal refactorings here to make things more clear. only request is in general smaller PR's are better. If moving code only do it separately as well. e.g. first move / re-org, then follow with changes, otherwise very hard to tell the diffs.

@jreback jreback added Output-Formatting __repr__ of pandas objects, to_string and removed Needs Triage Issue that has not been reviewed by a pandas team member labels Sep 16, 2020
@jreback jreback added this to the Contributions Welcome milestone Sep 16, 2020
@ivanovmg ivanovmg self-assigned this Sep 17, 2020
@mroeschke mroeschke removed this from the Contributions Welcome milestone Oct 13, 2022
@jbrockmendel
Copy link
Member

@attack68 i think you're the most qualified to comment on this: is this issue still actionable or has it been refactored as much as it reasonably can be?

@attack68
Copy link
Contributor

I removed the LatexFormatter and the to_latex method in #52844 , after re-routing DataFrame.to_latex via Styler.to_latex.

I think the end goal is really to do the same for HTML, i.e. rending it via Styler (and then remove HTMLFormatter), but it was difficult to get this through with disagreements on deprecation cycles and not be able to replicate every single keyword arg, etc.

If this did actually happen it would not leave this Issue with much to do really. So this Issue is probably still actionable but I think those actions might be quite different to what the OP actually suggests. Best to close?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Output-Formatting __repr__ of pandas objects, to_string Refactor Internal refactoring of code
Projects
None yet
Development

No branches or pull requests

5 participants