Adding support for pandas dataframes, multindex formatting #1046
Conversation
| @@ -380,6 +380,8 @@ Result: | |||
|
|
|||
|  | |||
|
|
|||
| This also works with index columns. Pass any integer to the `num_index_columns` argument when calling `Table()` and that many columns will be formatted according to the `index_style` argument. | |||
There was a problem hiding this comment.
I think this only applies to the panda adapter, and whould probably be removed.
However, it would be nice to add a section about the panda adapter to this file, as I'm sure many fpdf2 users would be happy to find out about it while reading this page 🙂
| from fpdf import FPDF | ||
|
|
||
|
|
||
| class FPDF_pandas(FPDF): |
There was a problem hiding this comment.
Given that this adapter only adds a single method, I think we should provide a mixin instead, so that fpdf2 users can combine several mixins if they want to!
class PandasMixin:
def dataframe(self, df, **kwargs):
...And that would be how end-users make use of it:
from fpdf import FPDF
from fpdf.pandas import PandasMixin
class MyPDF(FPDF, PandasMixin):
pass
pdf = MyPDF()
pdf.add_page()
pdf.set_font("Times", size=10)
pdf.dataframe(df, ...)What do you think of this approach @afriedman412 🙂?
| def __init__(self, **kwargs): | ||
| super().__init__(**kwargs) | ||
|
|
||
| def dataframe(self, df, **kwargs): |
There was a problem hiding this comment.
Some docstring would be nice before merging this PR 🙂 (as well as an addition in CHANGELOG.md)
| @@ -32,6 +33,7 @@ def __init__( | |||
| gutter_height=0, | |||
| gutter_width=0, | |||
| headings_style=DEFAULT_HEADINGS_STYLE, | |||
| index_style=DEFAULT_INDEX_STYLE, | |||
There was a problem hiding this comment.
New parameters should always be added at the end of the parameters list, otherwise the code of existing fpdf2 users would break if they are currently passing parameters by value, up to line_height for example there:
align = "CENTER"
v_align = "MIDDLE",
borders_layout = TableBordersLayout.ALL
cell_fill_color = None
cell_fill_mode = TableCellFillMode.NONE
col_widths = None
first_row_as_headings = True
gutter_height = 0
gutter_width = 0
headings_style = DEFAULT_HEADINGS_STYLE
line_height = None
with pdf.table(align, v_align, borders_layout, cell_fill_color, cell_fill_mode, col_widths, first_row_as_headings, gutter_height, gutter_width, headings_style, line_height) as table:
... # this code would break after merging this PR, because line_height would be passed to index_style
| @@ -40,6 +42,7 @@ def __init__( | |||
| padding=None, | |||
| outer_border_width=None, | |||
| num_heading_rows=1, | |||
| num_index_columns=0 | |||
There was a problem hiding this comment.
Given that those 2 new parameters are only required for rendering pandas dataframes, I think they should not be added there but in PandasMixin, even if that means overriding some Table methods in PandasMixin 🙂
|
Hi @afriedman412 👋 Thank you for submitting this PR 🙂 I made several comments. FYI, we are also considering adding some documentation (or even an adapter, similar to your approach there) on how to render |
Implements (partially) a solution for #970
table_pandas adapter code formats properly formats pandas dataframes
can now style table indexes automatically the way headers table headers are styled
The GitHub pipeline is OK (green),
meaning that both pylint (static code analyzer) and black (code formatter) are happy with the changes of this PR.
A unit test is covering the code added / modified by this PR
This PR is ready to be merged
In case of a new feature, docstrings have been added, with also some documentation in the docs/ folder
A mention of the change is present in CHANGELOG.md
By submitting this pull request, I confirm that my contribution is made under the terms of the GNU LGPL 3.0 license.