BUG: on_bad_lines=callable does not invoke callable for all bad lines

[indigoviolet]

Pandas version checks

• I have checked that this issue has not already been reported.

• I have confirmed this bug exists on the latest version of pandas.

• I have confirmed this bug exists on the main branch of pandas.

Reproducible Example

In [29]:
import pandas as pd
pd.__version__
Out [29]:
'1.4.3'

In [30]:
len(open("bad.csv").readlines())
Out [30]:
3

In [31]:
df1 = pd.read_csv("bad.csv", on_bad_lines='warn', engine='python')
Skipping line 3: ',' expected after '"'


In [32]:
df2 = pd.read_csv("bad.csv", on_bad_lines=print, engine='python')

In [33]:
len(df1), len(df2)
Out [33]:
(1, 1)

Issue Description

The above data file has two rows + header. Row 2 is valid, Row 3 is bad.

For df1, I'm setting on_bad_line=warn, and I see a warning for line 3.

For d2, I'm passing on_bad_lines=print, and I don't see any prints - the bad line is silently skipped.

❯ cat bad.csv
country,founded,id,industry,linkedin_url,locality,name,region,size,website
united states,"",heritage-equine-equipment-llc,farming,linkedin.com/company/heritage-equine-equipment-llc,"",heritage equine equipment llc,"",1-10,heritageequineequip.com
chile,"",contacto-corporación-colina,hospital & health care,linkedin.com/company/contacto-corporación-colina,colina,"contacto \" corporación colina",santiago metropolitan,11-50,corporacioncolina.cl

Expected Behavior

I would expect the bad line to be printed in the second case.

Installed Versions

pd.show_versions()

INSTALLED VERSIONS

commit : e8093ba
python : 3.9.12.final.0
python-bits : 64
OS : Linux
OS-release : 5.11.0-49-generic
Version : #55-Ubuntu SMP Wed Jan 12 17:36:34 UTC 2022
machine : x86_64
processor : x86_64
byteorder : little
LC_ALL : None
LANG : en_US.UTF-8
LOCALE : en_US.UTF-8

pandas : 1.4.3
numpy : 1.23.1
pytz : 2022.1
dateutil : 2.8.2
setuptools : 60.6.0
pip : 22.0.3
Cython : None
pytest : None
hypothesis : None
sphinx : None
blosc : None
feather : None
xlsxwriter : None
lxml.etree : None
html5lib : None
pymysql : None
psycopg2 : None
jinja2 : None
IPython : 8.4.0
pandas_datareader: None
bs4 : None
bottleneck : None
brotli : None
fastparquet : None
fsspec : None
gcsfs : None
markupsafe : None
matplotlib : None
numba : None
numexpr : None
odfpy : None
openpyxl : None
pandas_gbq : None
pyarrow : None
pyreadstat : None
pyxlsb : None
s3fs : None
scipy : None
snappy : None
sqlalchemy : None
tables : None
tabulate : None
xarray : None
xlrd : None
xlwt : None
zstandard : None
/home/venky/dev/instant-science/explore/.venv/lib/python3.9/site-packages/_distutils_hack/init.py:30: UserWarning: Setuptools is replacing distutils.
warnings.warn("Setuptools is replacing distutils.")

[phofl]

Hi, thanks for your report, can reproduce this too.

could you try simplifying the csv file? It’s hard to see what’s going on in there right now

[mroeschke]

This may be working as expected if I am looking at your csv file correctly.

As the docs state:

Specifies what to do upon encountering a bad line (a line with too many fields).

And I think each line has the same number of elements?

[indigoviolet]

1. Which lines are considered bad should not be different between 'warn' and print.

2. I would expect all skipped lines to be denoted bad, and for the callable to be able to handle all of them.

[indigoviolet]

Hi, thanks for your report, can reproduce this too.

could you try simplifying the csv file? It’s hard to see what’s going on in there right now

Here's a simplified version:

❯ cat bad2.csv
country,name
united states,heritage equine equipment llc
chile,"contacto \" corporación colina"

Setting escapechar='\\' will allow reading the second line, but the bug (different behavior b/w warn and print) as reported is still valid.

[baodangpro]

Agree that it should be a bug. No confidence to pass a callback for on_bad_line as some bad lines due to escapechar will be skipped silently.

[kostyafarber]

print returns None, so by the description in the docs the bad line will be ignored.

If the function returns None the bad line will be ignored

[kostyafarber]

So I think I've identified the issue:

pandas/pandas/io/parsers/python_parser.py

Lines 776 to 786 in eb23512

	try:
	# assert for mypy, data is Iterator[str] or None, would error in next
	assert self.data is not None
	line = next(self.data)
	# for mypy
	assert isinstance(line, list)
	return line
	except csv.Error as e:
	if self.on_bad_lines in (
	self.BadLineHandleMethod.ERROR,
	self.BadLineHandleMethod.WARN,

This bit here catches the CSV error (i.e in the example this line chile,"contacto \" corporación colina" not having defined an escapechar=\\) causes this error to be caught:

'\',\' expected after \'"\''

This block does not deal with user defined callables, only the pandas defined ones, and just returns None, effectively "skipping" this line.

The callable gets triggered in this snippet if the number of rows is not as expected:

pandas/pandas/io/parsers/python_parser.py

Lines 988 to 1008 in eb23512

	if (
	max_len > col_len
	and self.index_col is not False # type: ignore[comparison-overlap]
	and self.usecols is None
	):
	footers = self.skipfooter if self.skipfooter else 0
	bad_lines = []
	iter_content = enumerate(content)
	content_len = len(content)
	content = []
	for (i, l) in iter_content:
	actual_len = len(l)
	if actual_len > col_len:
	if callable(self.on_bad_lines):
	new_l = self.on_bad_lines(l)
	if new_l is not None:
	content.append(new_l)

For example if I append

japan, bum ba boom, doon

to the example csv

country,name
united states,heritage equine equipment llc
chile,"contacto \" corporación colina"
japan, bum ba boom, doon

The callable will trigger for this line, as max_len > col_len, but not for the chile,"contacto \" corporación colina" .

Not sure what the best way to go about a fix is. Perhaps we could call the user defined callable when we catch the error and let it process the line at return it?

Not sure what the implications would be downstream if the line wasn't processed correctly though and if we would want this to propagate down the parser.

Happy to work on this and open a PR if we have an idea on the approach for the fix.

[kostyafarber]

Any ideas?

[Dr-Irv]

Given how it's documented, I think this statement is correct:

I would expect all skipped lines to be denoted bad, and for the callable to be able to handle all of them.

In other words, the callable should be called if the line was detected as bad. That is what should get fixed.

@mroeschke do you agree that if the line is bad in any way, the callable should be called, as opposed to just getting called if the number of fields is wrong?

[mroeschke]

I think when implementing this feature at the time, I tailored the callable to apply to what a "bad line" was documented at the time

Specifies what to do upon encountering a bad line (a line with too many fields).

And relied on how too many fields was defined internally.

Not sure why too many fields was the definition of a bad line at the time, but I would be open to expanding the definition of what "bad" mean in regards to a line