Skip to content

Add support for hf datasets summarization#589

Merged
JulesBelveze merged 14 commits intorelease/1.1.0from
add-support-for-hf-datasets-summarization
Jul 6, 2023
Merged

Add support for hf datasets summarization#589
JulesBelveze merged 14 commits intorelease/1.1.0from
add-support-for-hf-datasets-summarization

Conversation

@Prikshit7766
Copy link
Copy Markdown
Contributor

@Prikshit7766 Prikshit7766 commented Jul 4, 2023

This PR aims on Adding Support of HF dataset for summarization task.

Checklist:

  • I've added Google style docstrings to my code.
  • I've used pydantic for typing when/where necessary.
  • I have linted my code
  • I have added tests to cover my changes.

@Prikshit7766 Prikshit7766 added the v2.1.0 Issue or request to be done in v2.1.0 release label Jul 4, 2023
@Prikshit7766 Prikshit7766 linked an issue Jul 4, 2023 that may be closed by this pull request
@Prikshit7766 Prikshit7766 requested a review from ArshaanNazir July 4, 2023 18:29
@RakshitKhajuria RakshitKhajuria added the ⭐ Feature Indicates new feature requests label Jul 4, 2023
@Prikshit7766 Prikshit7766 requested a review from JulesBelveze July 5, 2023 05:27
Copy link
Copy Markdown
Contributor

@JulesBelveze JulesBelveze left a comment

Choose a reason for hiding this comment

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

Few comments:

  1. It would be better to pass a parameter task when instantiating the HuggingFaceDataset. This will go in hand with the others *Dataset.
  2. It would be great to have a HuggingFaceData.load_data method that will then redirect to the loading method according to the task. That way we will always call the same function from the Harness.
  3. Please add SummarizationSample to the TypeVar Sample

@Prikshit7766 Prikshit7766 requested a review from JulesBelveze July 5, 2023 10:52
@RakshitKhajuria
Copy link
Copy Markdown
Contributor

Few comments:

  1. It would be better to pass a parameter task when instantiating the HuggingFaceDataset. This will go in hand with the others *Dataset.
  2. It would be great to have a HuggingFaceData.load_data method that will then redirect to the loading method according to the task. That way we will always call the same function from the Harness.
  3. Please add SummarizationSample to the TypeVar Sample

Thanks for the review @JulesBelveze :)

Copy link
Copy Markdown
Contributor

@JulesBelveze JulesBelveze left a comment

Choose a reason for hiding this comment

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

LGTM

@JulesBelveze JulesBelveze merged commit 757593d into release/1.1.0 Jul 6, 2023
@JulesBelveze JulesBelveze deleted the add-support-for-hf-datasets-summarization branch July 6, 2023 05:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

⭐ Feature Indicates new feature requests v2.1.0 Issue or request to be done in v2.1.0 release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add support for HF datasets (summarization)

3 participants