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

Add evaluation and document conversion to tutorial 15 #2325

Merged
merged 16 commits into from
Mar 29, 2022

Conversation

MichelBartels
Copy link
Contributor

@MichelBartels MichelBartels commented Mar 17, 2022

Proposed changes:
This adds an evaluation and a document conversion segment to tutorial 15. For this, it was necessary to change the dataset to a dataset with labels.

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@ZanSara ZanSara marked this pull request as draft March 21, 2022 10:45
@@ -95,7 +95,7 @@ Just as text passages, tables are represented as `Document` objects in Haystack.
from haystack.utils import fetch_archive_from_http

doc_dir = "data"
s3_url = "https://s3.eu-central-1.amazonaws.com/deepset.ai-farm-qa/datasets/documents/ottqa_sample.zip"
s3_url = "https://s3.eu-central-1.amazonaws.com/deepset.ai-farm-qa/datasets/documents/table_text_dataset.zip"
Copy link
Member

@julian-risch julian-risch Mar 21, 2022

Choose a reason for hiding this comment

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

@MichelBartels Once the dataset is uploaded, please replace the line in the dictionary here:

"https://s3.eu-central-1.amazonaws.com/deepset.ai-farm-qa/datasets/documents/ottqa_tables_sample.json.zip": "15",

@MichelBartels MichelBartels marked this pull request as ready for review March 28, 2022 14:08
@@ -88,7 +88,7 @@
},
Copy link
Contributor

Choose a reason for hiding this comment

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

This output should be removed.


Reply via ReviewNB

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

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.

Thanks for working on this, looking already really good to me. Before merging, it would be nice to see the following points addressed:

  • Please remove the KeyboardInterrupt output from the notebook
  • It would be nice to have the outputs of the print methods in the notebook. Like this, the user doesn't need to run the notebook in order to understand what the outcome of the different sections is.
  • The Evaluation section is missing in the .py-file
  • Please add labels to the PR.

@MichelBartels
Copy link
Contributor Author

Thanks @bogdankostic, I have made your suggested changes.

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

@MichelBartels MichelBartels merged commit eb514a6 into master Mar 29, 2022
@MichelBartels MichelBartels deleted the table_eval_tutorial branch March 29, 2022 15:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic:tableQA type:documentation Improvements on the docs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants