core: fix CommaSeparatedListOutputParser to handle columns that may contain commas in it

[jkyamog]

• Description:
  Currently CommaSeparatedListOutputParser can't handle strings that may contain commas within a column. It would parse any commas as the delimiter.
  Ex.
  "foo, foo2", "bar", "baz"

It will create 4 columns: "foo", "foo2", "bar", "baz"

This should be 3 columns:

"foo, foo2", "bar", "baz"

• Dependencies:
  Added 2 additional imports, but they are built in python packages.

import csv
from io import StringIO

• Twitter handle: @jkyamog

• Add tests and docs:

1. added simple unit test test_multiple_items_with_comma

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
langchain	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Sep 17, 2024 9:35pm

[efriis]

Hey there! This is a breaking change in certain cases. Would it be possible to only use the new csv-based parsing logic in the event some flag is set on the output parser? e.g. CommaSeparatedListOutputParser(parse_quoted_cells=True)

[jkyamog]

Hey there! This is a breaking change in certain cases. Would it be possible to only use the new csv-based parsing logic in the event some flag is set on the output parser? e.g. CommaSeparatedListOutputParser(parse_quoted_cells=True)

Yes I can definitely just make it a flag. I didn't realize it was breaking other use cases. I did run the test on this, and I added 1 additional test. So my understanding it covered the previous cases. Before adding a new flag, I would like to see if it's possible to cover for all identified use cases? Can you give me an example where it gets broken?

For you convenience here are the current tests are:

test_single_item
    text = "foo"
    expected = ["foo"]

test_multiple_items_with_spaces
    text = "foo, bar, baz"
    expected = ["foo", "bar", "baz"]

test_multiple_items
    text = "foo,bar,baz"
    expected = ["foo", "bar", "baz"]

test_multiple_items_with_comma <- new test that I added based on the previous existing one.
    text = '"foo, foo2",bar,baz'
    expected = ["foo, foo2", "bar", "baz"]

Can you tell me what cases where it fails?

text = "FAILING CASE"
expected = ["foo", "bar" .....]

[efriis]

test_multiple_items_with_comma_existing_behavior
text = '" is a double quote character,another double quote character is ",this cell has no double quote'
expected = ['"this is a double quote character', 'another double quote character is "', 'this cell has no double quote']

note that the behavior will be different using the new flag (then it will match your test)

[jkyamog]

test_multiple_items_with_comma_existing_behavior text = '" is a double quote character,another double quote character is ",this cell has no double quote' expected = ['"this is a double quote character', 'another double quote character is "', 'this cell has no double quote']

note that the behavior will be different using the new flag (then it will match your test)

Sorry I only looked at this right now. Adding the flag on class level CommaSeparatedListOutputParser(parse_quoted_cells=True) it gives a lot of issues of existing test/code can't seem to instantiate it. Not sure, but maybe because it is being serialized and pydantic is not aware of the attribute parse_quoted_cells?

I then tried to see if I can just add the flag on the method level.

    assert parser.parse(text,parse_quoted_cells=True) == expected
    assert add(parser.transform(t for t in text)) == expected

This works on the .parse, but fails on .transform. I am not sure what is the purpose of .transform, I guess that is how its called down the chain? Appreciate some guidance on this, my initial thoughts that this would be simple change and even keeping the original behavior but I might be missing something.