bug: change type of split_by to Literal including None

[julian-risch]

Related Issues

• fixes Bug: Load PreProcessor with split_by: None from config fails #3386

Proposed Changes:

This PR changes the type annotation of split_by from str to Literal["word", "sentence", "passage", None].
The type of the split_by parameter indicated here was str but it should be Optional[str] or even more explicitly be Literal["word", "sentence", "passage", None] instead: https://github.com/deepset-ai/haystack/blob/main/haystack/nodes/preprocessor/preprocessor.py#L54 and the schema file generated based on that with the following line should also allow None as the type of split_by then:

haystack/haystack/json-schemas/haystack-pipeline-main.schema.json

Line 4286 in 7290196

"type": "string"

Checklist

• I have read the contributors guidelines and the code of conduct
• I have updated the related issue with new insights and changes
• I added tests that demonstrate the correct behavior of the change
• I've used the conventional commit convention for my PR title
• I documented my code
• I ran pre-commit hooks and fixed any issue

[sjrl]

Hey, @julian-risch I realize this is still in draft, but I'd like to ask a quick question. Would it make sense to make this option a Literal like Literal["word", "sentence", "passage", None] in this scenario? I see we do that in other parts of Haystack like here. However, I'm not sure if using a Literal here would still cause errors in the config validation so I don't know if it is relevant here. Any insight on this would be appreciated!

[julian-risch]

Hi @sjrl not a bad idea actually. 🙂 I was just thinking about a quick fix for the issue reported by a community member but a Literal would be a cleaner solution. I'll do that instead. Thanks for the hint. 👍

[julian-risch]

@masci This PR is ready for review. 🙂

[masci]

LGTM 👍