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

Passing the meta-data in the summarizer response #2179

Merged
merged 7 commits into from Jul 11, 2022

Conversation

SjSnowball
Copy link
Contributor

@SjSnowball SjSnowball commented Feb 13, 2022

PR for #2052
Currently, the summarizer only returns the "context" (the original content for summerization) field and summarized content field. So have added the code snippet which will send the "context" field, summarized content field. and also the other meta-data fields.

Status (please check what you already did):

  • First draft (up for discussions & feedback)
  • Final code
  • Added tests
  • Updated documentation

@ZanSara

@SjSnowball SjSnowball changed the title Passing the meta-data in the summerizer Passing the meta-data in the summerizer response Feb 13, 2022
Copy link
Contributor

@ZanSara ZanSara left a comment

Choose a reason for hiding this comment

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

Hey @SjSnowball, thank you so much for your contribution! I left a few comments because I believe there's a parameter of predict() you haven't noticed and might make your code behave unexpectedly.

I'm up for a discussion about the best approach, feel free to try different alternatives!

haystack/nodes/summarizer/transformers.py Outdated Show resolved Hide resolved
test/test_summarizer.py Outdated Show resolved Hide resolved
test/test_summarizer.py Outdated Show resolved Hide resolved
@ZanSara ZanSara added topic:metadata type:feature New feature or request labels Feb 14, 2022
@ZanSara
Copy link
Contributor

ZanSara commented Apr 12, 2022

Hey @SjSnowball, do you think you will have time for this PR? Otherwise I can take care of completing and merging it myself 👍

@julian-risch
Copy link
Member

Hi @SjSnowball this pull request looks quite good already! 👍 We plan another Haystack release for next week and it would be great if we could include this pull request in the release. Do you think you could split the test into two separate tests as suggested by @ZanSara and address the other two change requests as well?

@SjSnowball
Copy link
Contributor Author

@julian-risch @ZanSara I will do the changes and push.

@ZanSara
Copy link
Contributor

ZanSara commented Jun 15, 2022

Hey @SjSnowball, can I help you with this PR? I'm afraid it's getting stale, but I can take over, fix the last things and merge if you don't have time anymore. Let us know 🙂

@ZanSara ZanSara merged commit 4d8f404 into deepset-ai:master Jul 11, 2022
Krak91 pushed a commit to Krak91/haystack that referenced this pull request Jul 26, 2022
* Passing the all the meta-data in the summerizer

* Disable metadata forwarding if `generate_single_summary` is `True`

* Update Documentation & Code Style

* simplify tests

* Update Documentation & Code Style

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@tstadel tstadel changed the title Passing the meta-data in the summerizer response Passing the meta-data in the summarizer response Aug 11, 2022
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.

None yet

4 participants