Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove **kwargs from FARMReader #2413

Merged
merged 6 commits into from
Apr 14, 2022

Conversation

ZanSara
Copy link
Contributor

@ZanSara ZanSara commented Apr 13, 2022

Problem
See #2362

Solution
This PR simply removes the kwargs from FARMReader. An earlier attempt to keep the functionality and remove the usage from kwargs at deeper layer failed, so I believe for now this is the only viable option. Unfortunately, this technically constitutes as a breaking change.

@ZanSara ZanSara removed the request for review from bogdankostic April 13, 2022 10:10
@ZanSara ZanSara marked this pull request as draft April 13, 2022 10:10
@ZanSara ZanSara changed the base branch from master to yaml_kwargs_in_custom_node April 13, 2022 11:21
@ZanSara ZanSara changed the base branch from yaml_kwargs_in_custom_node to master April 13, 2022 11:26
tstadel and others added 4 commits April 13, 2022 13:35
…odes (#2350)

* Add tests for missing init and super

* Update Documentation & Code Style

* change in with endswith

* Move test in pipeline.py and change test in pipeline_yaml.py

* Update Documentation & Code Style

* Use caplog to test the warning

* Update Documentation & Code Style

* move tests into test_pipeline and use get_config

* Update Documentation & Code Style

* Unmock version name

* Update Documentation & Code Style

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
* Update linux_ci.yml
@ZanSara ZanSara marked this pull request as ready for review April 13, 2022 17:08
Copy link
Contributor

@bogdankostic bogdankostic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ZanSara ZanSara changed the base branch from master to yaml_kwargs_in_custom_node April 14, 2022 11:04
@ZanSara ZanSara merged commit c3e9a44 into yaml_kwargs_in_custom_node Apr 14, 2022
@ZanSara ZanSara deleted the remove_kwargs_from_farmreader2 branch April 14, 2022 11:06
ZanSara added a commit that referenced this pull request Apr 14, 2022
* Add failing test

* Remove `**kwargs` from docstores' `__init__` functions (#2407)

* Remove kwargs from ESDocStore subclasses

* Remove kwargs from subclasses of SQLDocumentStore

* Remove kwargs from Weaviate

* Revert change in pinecone

* Fix tests

* Fix retriever test wirh weaviate

* Change Exception into DocumentStoreError

* Update Documentation & Code Style

* Remove `**kwargs` from `FARMReader` (#2413)

* Remove FARMReader kwargs without trying to replace them functionally

* Update Documentation & Code Style

* enforce same index values before and after saving/loading eval dataframes (#2398)

* Add tests for missing `__init__` and `super().__init__()` in custom nodes (#2350)

* Add tests for missing init and super

* Update Documentation & Code Style

* change in with endswith

* Move test in pipeline.py and change test in pipeline_yaml.py

* Update Documentation & Code Style

* Use caplog to test the warning

* Update Documentation & Code Style

* move tests into test_pipeline and use get_config

* Update Documentation & Code Style

* Unmock version name

* Improve variadic args test

* Update Documentation & Code Style

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants