refactor: improve support for dataclasses

[danielbichuetti]

Related Issues

• fixes Remove pin from pydantic #3128

Proposed Changes:

The latest update on pydantic caused some tests to fail. The main reason is related to some initialization pydantic do into dataclasses for his own internal logic. These extra fields were being mutated to Document.meta in reason of its logic. To avoid this error, I have filtered the new pydantic current initialization variable and also the old one (<1.10). This will make it more consistent.

Furthermore, I've refactored class initialization to start the usage of the dataclasses features. The custom logic has been moved to private post_init method.

UPDATE: because of some differences at dataclasses initialization in Python 3.7 and 3.9, code had to be refatored and the exclusive usage of post_init has been dropped. Using custom init and InitVar worked on supported environments.

How did you test it?

Currently making complete tests

Notes for the reviewer

Checklist

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

[danielbichuetti]

Current code need refactoring due to Optional arg and mypy. I'll push new commit.

[ZanSara]

Hey @danielbichuetti, wow thank you! 🤩 Can I get started with the review already or you prefer me to wait for your fixes for mypy and typing?

[danielbichuetti]

Hi @ZanSara. I will revert the initialization changes, so mypy results are ok with pydantic, and code can get checked by other tests.

I've started to work on some refactoring, looking for a better mypy integration.

As a side note, am I the only one getting issues with VS Code and pre-commits?

[danielbichuetti]

Ok, beyond the mypy checking, there are differences when running tests on Python 3.9 and the CI which uses 3.7. I'm looking at what causes the errors in the different environments.

As a complement for further testing:

============================= test session starts ==============================
platform linux -- Python 3.9.13, pytest-7.1.2, pluggy-1.0.0
rootdir: /home/danielbichuetti/Dev/haystack, configfile: pyproject.toml
plugins: anyio-3.6.1, custom-exit-code-0.3.0, typeguard-2.13.3
collected 18 items

test/others/test_schema.py::test_no_answer_label PASSED                  [  5%]
test/others/test_schema.py::test_equal_label PASSED                      [ 11%]
test/others/test_schema.py::test_answer_to_json PASSED                   [ 16%]
test/others/test_schema.py::test_answer_to_dict PASSED                   [ 22%]
test/others/test_schema.py::test_label_to_json PASSED                    [ 27%]
test/others/test_schema.py::test_label_to_dict PASSED                    [ 33%]
test/others/test_schema.py::test_doc_to_json PASSED                      [ 38%]
test/others/test_schema.py::test_answer_postinit PASSED                  [ 44%]
test/others/test_schema.py::test_generate_doc_id_using_text PASSED       [ 50%]
test/others/test_schema.py::test_generate_doc_id_using_custom_list PASSED [ 55%]
test/others/test_schema.py::test_aggregate_labels_with_labels PASSED     [ 61%]
test/others/test_schema.py::test_multilabel_preserve_order PASSED        [ 66%]
test/others/test_schema.py::test_multilabel_preserve_order_w_duplicates PASSED [ 72%]
test/others/test_schema.py::test_multilabel_id PASSED                    [ 77%]
test/others/test_schema.py::test_serialize_speech_document PASSED        [ 83%]
test/others/test_schema.py::test_deserialize_speech_document PASSED      [ 88%]
test/others/test_schema.py::test_serialize_speech_answer PASSED          [ 94%]
test/others/test_schema.py::test_deserialize_speech_answer PASSED        [100%]

=============================== warnings summary ===============================
venv/lib/python3.9/site-packages/transformers/image_utils.py:222
  /home/danielbichuetti/Dev/haystack/venv/lib/python3.9/site-packages/transformers/image_utils.py:222: DeprecationWarning: BILINEAR is deprecated and will be removed in Pillow 10 (2023-07-01). Use Resampling.BILINEAR instead.
    def resize(self, image, size, resample=PIL.Image.BILINEAR, default_to_square=True, max_size=None):

-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html
-------------- generated xml file: /tmp/tmp-17508cX6nXSA55Gqu.xml --------------
======================== 18 passed, 1 warning in 2.37s =========================

And on CI, Python 3.7:

=================================== FAILURES ===================================
____________________ test_generate_doc_id_using_custom_list ____________________

    def test_generate_doc_id_using_custom_list():
        text1 = "text1"
        text2 = "text2"
    
        doc1_meta1_id_by_content = Document(content=text1, meta={"name": "doc1"}, id_hash_keys=["content"])
        doc1_meta2_id_by_content = Document(content=text1, meta={"name": "doc2"}, id_hash_keys=["content"])
        assert doc1_meta1_id_by_content.id == doc1_meta2_id_by_content.id
    
        doc1_meta1_id_by_content_and_meta = Document(content=text1, meta={"name": "doc1"}, id_hash_keys=["content", "meta"])
        doc1_meta2_id_by_content_and_meta = Document(content=text1, meta={"name": "doc2"}, id_hash_keys=["content", "meta"])
>       assert doc1_meta1_id_by_content_and_meta.id != doc1_meta2_id_by_content_and_meta.id
E       AssertionError: assert '4f7944b21e062f189b8c19e9293c7602' != '4f7944b21e062f189b8c19e9293c7602'
E        +  where '4f7944b21e062f189b8c19e9293c7602' = <Document: {'content': 'text1', 'content_type': 'text', 'score': None, 'meta': {'name': 'doc1'}, 'embedding': None, 'id': '4f7944b21e062f189b8c19e9293c7602'}>.id
E        +  and   '4f7944b21e062f189b8c19e9293c7602' = <Document: {'content': 'text1', 'content_type': 'text', 'score': None, 'meta': {'name': 'doc2'}, 'embedding': None, 'id': '4f7944b21e062f189b8c19e9293c7602'}>.id

test/others/test_schema.py:198: AssertionError

[ZanSara]

As a side note, am I the only one getting issues with VS Code and pre-commits?

Which kind of issues? We've had in the past but I hoped they were mostly solved by now. Let's open an issue for this topic: even though it might be a simple misconfiguration, it's always helpful to leave an issue with the discussion for other users

[ZanSara]

Ok, beyond the mypy checking, there are differences when running tests on Python 3.9 and the CI which uses 3.7. I'm looking at what causes the errors in the different environments.

Alright! Given that there are still some fixes to be done, I'll set this one to draft until the CI looks good again

[danielbichuetti]

Tests are failing due to timeouts into haystack CI. At the same time, I have run them using my profile. All have run ok: https://github.com/danielbichuetti/haystack/actions/runs/2982709078

I'll force one extra commit tomorrow, maybe CI run correctly.

[danielbichuetti]

May someone check CI please ? It has skipped some tests consistently.

[masci]

The HF models caching fails quite often, besides the timeouts on the download process the problem is the cache shouldn't expire so often, I wonder if we're hitting the 10Gb size limit. Opened #3146 to track that, in the meantime re-running the job did the trick here ✅

[ZanSara]

Hey @danielbichuetti! Thanks a ton for this PR! Sorry if I've been a bit overzealous with the review, but this is a delicate change, so it's better to be extra-careful. Good news is, there are no big issues with the changes, it's just many small improvements.

Let me know if you have any question or if I got anything wrong 🙂

[ZanSara]

Maybe worth pinning pydantic<2 to avoid having the same problems all over again when Pydantic 2.0 comes out? @masci

[danielbichuetti]

IMHO, I don't think it's necessary.

Here you can see what will be removed: https://pydantic-docs.helpmanual.io/blog/pydantic-v2/#removed-features-limitations.

The dataclasses will only have internal changes, not on the external interface. https://pydantic-docs.helpmanual.io/blog/pydantic-v2/#features-remaining

Furthermore, pydantic v2 will have a performance increase of at least 4x. So, its worth using it.

[ZanSara]

Looking good! Thanks again 😊

[danielbichuetti]

@ZanSara I think you approved and forgot to merge 😆

[ZanSara]

Thank you for the ping!! 😄 I did forget to merge it 😄