fix: strip whitespaces safely from FARMReader's answers

[ZanSara]

Related Issues

• fixes End of search context not pointing to the end of a phrase #3504

Proposed Changes:

• Remove mindless .strip() call in _span_to_string that was messing with the token offsets for FARMReader.
• Strip right-side whitespace safely and adjust the start char position when necessary

How did you test it?

• CI

Notes for the reviewer

n/a

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

[julian-risch]

The changes to the test case look good to me but I have a few questions about the data type of right_offset and how it is used for calculations.

[julian-risch]

I agree with the change from rstrip to lstrip. 👍 Regarding the int() call and the right_offest I still need to be convinced of the change. 🙂 I think we can leave out the int() call and I would rename right_offset to left_offset.

[julian-risch]

I can confirm that prediction isn't used in any other tests except for the reader.py. However, it might be useful to have this fixture available to other nodes as well no?

[ZanSara]

I don't think so. I think fixtures should stay close to the tests that use them, and this is such a small one I hardly see the point of having it at all. Let's keep it near the tests.

[julian-risch]

Looks good to me! 👍 Thank you for patiently addressing the change requests and for the quick responses in our discussions. 🙂

[ZanSara]

Thank you for catching them! 🙌