Skip to content

Commit 7c675c1

Browse files
authored
fix(api,robot-server): let CSVParameter dialect sniffer see entire CSV file (#20245)
# Overview A customer reported that a parameterized protocol works with a short CSV file but fails with their longer CSV file: https://opentrons.slack.com/archives/C389UCULX/p1763376654727119 The root cause seems to be that in `parse_as_csv()`, we were truncating the CSV file to 1024 bytes before passing it to the `csv.Sniffer`. If that chops up a line at an inopportune place, it would cause the sniffer to fail with `Could not determine delimiter`. We should just pass the whole CSV file to the sniffer. From our meeting this morning, we said we would fix this in `edge` rather than trying to get it into RS 8.8.0. ## Test Plan and Hands on Testing Added a test case with a long CSV file derived from the customer report. ## Risk assessment Low. I guess there's a small risk that there could be some horrifying junk in the CSV file after the first 1024 bytes, that the sniffer previously wouldn't see, that would now be visible to the sniffer. But I think overall, it's more correct to let the sniffer see the whole file, rather than arbitrarily cutting it off at 1024 bytes.
1 parent 6e2f037 commit 7c675c1

File tree

2 files changed

+31
-1
lines changed

2 files changed

+31
-1
lines changed

api/src/opentrons/protocols/parameters/csv_parameter_interface.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ def parse_as_csv(
7070
rows: List[List[str]] = []
7171
if detect_dialect:
7272
try:
73-
dialect = csv.Sniffer().sniff(self.contents[:1024])
73+
dialect = csv.Sniffer().sniff(self.contents)
7474
reader = csv.reader(self.contents.split("\n"), dialect, **kwargs)
7575
except (UnicodeDecodeError, csv.Error):
7676
raise ParameterValueError(

api/tests/opentrons/protocols/parameters/test_csv_parameter_interface.py

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,23 @@ def csv_file_different_delimiter() -> bytes:
4545
return b"x:y:z\na,:1,:2\nb,:3,:4\nc,:5,:6"
4646

4747

48+
@pytest.fixture()
49+
def csv_file_long() -> bytes:
50+
"""A long CSV file from a customer that caused the sniffer to fail when it only looked at the first 1024 bytes."""
51+
return b"""
52+
Source Labware,Source Slot,Source Well,Source Height,Dest Labware,Dest Slot,Dest Well,Volume
53+
opentrons_10_tuberack_falcon_4x50ml_6x15ml_conical,0,A1,0,opentrons_24_aluminumblock_nest_0.5ml_screwcap,0,A1,100
54+
opentrons_10_tuberack_falcon_4x50ml_6x15ml_conical,0,A1,0,opentrons_24_aluminumblock_nest_0.5ml_screwcap,0,A1,100
55+
opentrons_10_tuberack_falcon_4x50ml_6x15ml_conical,0,A1,0,opentrons_24_aluminumblock_nest_0.5ml_screwcap,0,A1,100
56+
opentrons_10_tuberack_falcon_4x50ml_6x15ml_conical,0,A1,0,opentrons_24_aluminumblock_nest_0.5ml_screwcap,0,A1,100
57+
opentrons_10_tuberack_falcon_4x50ml_6x15ml_conical,0,A1,0,opentrons_24_aluminumblock_nest_0.5ml_screwcap,0,A1,100
58+
opentrons_10_tuberack_falcon_4x50ml_6x15ml_conical,0,A1,0,opentrons_24_aluminumblock_nest_0.5ml_screwcap,0,A1,100
59+
opentrons_10_tuberack_falcon_4x50ml_6x15ml_conical,0,A1,0,opentrons_24_aluminumblock_nest_0.5ml_screwcap,0,A1,100
60+
opentrons_10_tuberack_falcon_4x50ml_6x15ml_conical,0,A1,0,opentrons_24_aluminumblock_nest_0.5ml_screwcap,0,A1,100
61+
opentrons_10_tuberack_falcon_4x50ml_6x15ml_conical,0,A1,0,opentrons_24_aluminumblock_nest_0.5ml_screwcap,0,A1,100
62+
""".strip()
63+
64+
4865
@pytest.fixture
4966
def csv_file_basic_trailing_empty() -> Tuple[bytes, List[List[str]]]:
5067
"""A basic CSV file with quotes around strings and a trailing newline."""
@@ -102,6 +119,19 @@ def test_csv_parameter(
102119
assert subject.parse_as_csv()[0] == ["x", "y", "z"]
103120

104121

122+
def test_csv_parameter_long_file(
123+
decoy: Decoy, api_version: APIVersion, csv_file_long: bytes
124+
) -> None:
125+
"""It should detect the CSV dialect for files of unlimited length."""
126+
# The previous implementation of parse_as_csv() passed only the first 1024 bytes of
127+
# the CSV file to the dialect sniffer, chopping up a line an unfortunate position,
128+
# causing the sniffer to fail with "Could not determine delimiter".
129+
subject = CSVParameter(csv_file_long, api_version)
130+
parsed_rows = subject.parse_as_csv()
131+
assert len(parsed_rows) == 10
132+
assert len(parsed_rows[0]) == 8
133+
134+
105135
@pytest.mark.parametrize(
106136
"csv_file",
107137
[

0 commit comments

Comments
 (0)