Skip to content

Avoid empty lines with spaces to be transformed to empty string #59155

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.

Sign up for GitHub

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
wants to merge 9 commits into from
Closed

Avoid empty lines with spaces to be transformed to empty string #59155

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
619fced
Fix issue 59147
ritwizsinha Jul 1, 2024
6eeb058
Remove useless prints
ritwizsinha Jul 1, 2024
0982da9
Fix ci
ritwizsinha Jul 1, 2024
e5032f3
Merged main
ritwizsinha Jul 1, 2024
f2dee95
Fix ci
ritwizsinha Jul 1, 2024
75e342a
Add tests
ritwizsinha Jul 2, 2024
461ebf0
merged main
ritwizsinha Jul 2, 2024
ece2f7b
Remove key argument skip_blank_lines and pass it implicitly
ritwizsinha Jul 2, 2024
e9b8530
Merge branch 'main' into Issue#59147
ritwizsinha Jul 4, 2024
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
1 change: 1 addition & 0 deletions doc/source/whatsnew/v3.0.0.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -563,6 +563,7 @@ I/O
- Bug in :meth:`read_csv` raising ``TypeError`` when ``index_col`` is specified and ``na_values`` is a dict containing the key ``None``. (:issue:`57547`)
- Bug in :meth:`read_csv` raising ``TypeError`` when ``nrows`` and ``iterator`` are specified without specifying a ``chunksize``. (:issue:`59079`)
- Bug in :meth:`read_excel` raising ``ValueError`` when passing array of boolean values when ``dtype="boolean"``. (:issue:`58159`)
- Bug in :meth:`read_html` would return an incorrect result when parsing a table with a space character in a ``<td>`` tag. (:issue:`12345`)
- Bug in :meth:`read_json` not validating the ``typ`` argument to not be exactly ``"frame"`` or ``"series"`` (:issue:`59124`)
- Bug in :meth:`read_stata` raising ``KeyError`` when input file is stored in big-endian format and contains strL data. (:issue:`58638`)

Expand Down
10 changes: 8 additions & 2 deletions pandas/io/html.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -507,6 +507,9 @@ def _expand_colspan_rowspan(

# Append the text from this <td>, colspan times
text = _remove_whitespace(self._text_getter(td))
if len(text) == 0:
text = self._text_getter(td)

if self.extract_links in ("all", section):
href = self._href_getter(td)
text = (text, href)
Expand Down Expand Up @@ -1027,6 +1030,7 @@ def read_html(
extract_links: Literal[None, "header", "footer", "body", "all"] = None,
dtype_backend: DtypeBackend | lib.NoDefault = lib.no_default,
storage_options: StorageOptions = None,
skip_blank_lines: bool = True,
Copy link
Member

Choose a reason for hiding this comment

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

How does this relate to fixing the bug? Generally to introduce new parameters to a function you'd have to open an enhancement issue.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This argument is needed because this function might need to specify the option whether they want to consider columns with just spaces as a valid column or not. If this is true, any column which is blank would be skipped as before. Hereis the place where this is checked to remove whitespaced lines

) -> list[DataFrame]:
r"""
Read HTML tables into a ``list`` of ``DataFrame`` objects.
Expand Down Expand Up @@ -1146,6 +1150,9 @@ def read_html(

.. versionadded:: 2.1.0

skip_blank_lines : bool, default True
Whether lines containing only spaces should be skipped or not.

Returns
-------
dfs
Expand Down Expand Up @@ -1201,9 +1208,7 @@ def read_html(

validate_header_arg(header)
check_dtype_backend(dtype_backend)

io = stringify_path(io)

return _parse(
flavor=flavor,
io=io,
Expand All @@ -1223,4 +1228,5 @@ def read_html(
extract_links=extract_links,
dtype_backend=dtype_backend,
storage_options=storage_options,
skip_blank_lines=skip_blank_lines,
)
44 changes: 44 additions & 0 deletions pandas/tests/io/test_html.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1242,6 +1242,50 @@ def test_preserve_empty_rows(self, flavor_read_html):

tm.assert_frame_equal(result, expected)

def test_preserve_rows_with_spaces(self, flavor_read_html):
result = flavor_read_html(
StringIO(
"""
<table>
<tr>
<th>A</th>
<th>B</th>
</tr>
<tr>
<td>a</td>
<td>b</td>
</tr>
<tr>
<td> </td>
<td> </td>
</tr>
</table>
"""
),
skip_blank_lines=False,
)[0]
expected = DataFrame(data=[["a", "b"], [" ", " "]], columns=["A", "B"])

tm.assert_frame_equal(result, expected)

def test_preserve_table_with_just_spaces(self, flavor_read_html):
result = flavor_read_html(
StringIO(
"""
<table>
<tr>
<td> </td>
</tr>
</table>
"""
),
skip_blank_lines=False,
Copy link
Member

Choose a reason for hiding this comment

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

Is this necessary? If it's left as the default then what will the result be?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Leaving it as default which is true would give the same behaviour as the bug, because if this is true, python_parser calls _remove_empty_lines which gets rid of lines with only spaces.

Copy link
Member

Choose a reason for hiding this comment

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

So running read_html without passing skip_blank_lines=False will produce an empty list, but the doc states that no empty list should be returned, so the bug isn't really fixed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see, so removing the new argument to parse_html, I can instead pass the skip_blank_lines to be False by default to _parse function to always get space only lines as a row instead of avoiding them

)[0]

expected = DataFrame(data=[" "])
assert len(result) != 0
tm.assert_frame_equal(result, expected)

def test_ignore_empty_rows_when_inferring_header(self, flavor_read_html):
result = flavor_read_html(
StringIO(
Expand Down
Loading