Skip to content

ENH: exclude html table border w/False value #45943

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

Merged
merged 21 commits into from
Apr 17, 2022
Merged
Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions doc/source/whatsnew/v1.5.0.rst
Original file line number Diff line number Diff line change
Expand Up @@ -566,6 +566,8 @@ I/O
- Bug in Parquet roundtrip for Interval dtype with ``datetime64[ns]`` subtype (:issue:`45881`)
- Bug in :func:`read_excel` when reading a ``.ods`` file with newlines between xml elements (:issue:`45598`)
- Bug in :func:`read_parquet` when ``engine="fastparquet"`` where the file was not closed on error (:issue:`46555`)
- :meth:`to_html` now excludes the ``border`` attribute from ``<table>`` elements when ``border`` keyword is set to ``False``.
-

Period
^^^^^^
Expand Down
2 changes: 1 addition & 1 deletion pandas/core/frame.py
Original file line number Diff line number Diff line change
Expand Up @@ -2892,7 +2892,7 @@ def to_html(
classes: str | list | tuple | None = None,
escape: bool = True,
notebook: bool = False,
border: int | None = None,
border: int | bool | None = None,
table_id: str | None = None,
render_links: bool = False,
encoding: str | None = None,
Expand Down
2 changes: 1 addition & 1 deletion pandas/io/formats/format.py
Original file line number Diff line number Diff line change
Expand Up @@ -1058,7 +1058,7 @@ def to_html(
encoding: str | None = None,
classes: str | list | tuple | None = None,
notebook: bool = False,
border: int | None = None,
border: int | bool | None = None,
table_id: str | None = None,
render_links: bool = False,
) -> str | None:
Expand Down
14 changes: 11 additions & 3 deletions pandas/io/formats/html.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ def __init__(
self,
formatter: DataFrameFormatter,
classes: str | list[str] | tuple[str, ...] | None = None,
border: int | None = None,
border: int | bool | None = None,
table_id: str | None = None,
render_links: bool = False,
) -> None:
Expand All @@ -57,8 +57,11 @@ def __init__(
self.bold_rows = self.fmt.bold_rows
self.escape = self.fmt.escape
self.show_dimensions = self.fmt.show_dimensions
if border is None:
if border is None or (border and isinstance(border, bool)):
border = cast(int, get_option("display.html.border"))
elif not border and isinstance(border, bool):
Copy link
Contributor

Choose a reason for hiding this comment

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

why is this instance check needed?

Copy link
Contributor Author

@z3c0 z3c0 Apr 10, 2022

Choose a reason for hiding this comment

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

Not my first choice, but it's to pass this test case:

(None, lambda df: df.to_html(border=0), "0"),

Setting border=0 has to result in border="0" being present in the output HTML. The instance check is to ensure that only a boolean can be used to hide the border.

Copy link
Contributor

Choose a reason for hiding this comment

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

why is border=0 not de-facto the same as border=False?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In my opinion, it should be. That's how I originally wrote the fix. My only reason for taking this approach is to conform to the test case, which explicitly tests for a border attribute to be present upon border=0.

Copy link
Contributor

Choose a reason for hiding this comment

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

i think ok to treat these cases as the same and can simplify the code a bit

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good to me - I will revert to my original fix, and update that test case instead

border = None

self.border = border
self.table_id = table_id
self.render_links = render_links
Expand Down Expand Up @@ -237,8 +240,13 @@ def _write_table(self, indent: int = 0) -> None:
else:
id_section = f' id="{self.table_id}"'

if self.border is None:
border_attr = ""
else:
border_attr = f' border="{self.border}"'

self.write(
f'<table border="{self.border}" class="{" ".join(_classes)}"{id_section}>',
f'<table{border_attr} class="{" ".join(_classes)}"{id_section}>',
indent,
)

Expand Down
19 changes: 19 additions & 0 deletions pandas/tests/io/test_html.py
Original file line number Diff line number Diff line change
Expand Up @@ -1148,6 +1148,25 @@ def test_to_html_timestamp(self):
result = df.to_html()
assert "2000-01-01" in result

def test_to_html_borderless(self):
df = DataFrame([{"A": 1, "B": 2}])
out_border_default = df.to_html()
out_border_true = df.to_html(border=True)
out_border_explicit_default = df.to_html(border=1)
out_border_nondefault = df.to_html(border=2)
out_border_zero = df.to_html(border=0)

out_border_false = df.to_html(border=False)

assert ' border="1"' in out_border_default
assert out_border_true == out_border_default
assert out_border_default == out_border_explicit_default
assert out_border_default != out_border_nondefault
assert ' border="2"' in out_border_nondefault
assert ' border="0"' in out_border_zero
assert " border" not in out_border_false
assert out_border_zero != out_border_false

@pytest.mark.parametrize(
"displayed_only,exp0,exp1",
[
Expand Down