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

Related #16 | Add Tree schema and CONSTITUENCY_PARSING task #295

Merged
merged 4 commits into from
Jan 9, 2024

Conversation

MJonibek
Copy link
Collaborator

@MJonibek MJonibek commented Jan 6, 2024

Related #16

Note:

  1. Add tree schema as was discussed here (with some small changes)
  2. Add CONSTITUENCY_PARSING task named CONST_PAR using Tree schema as was discussed here

Copy link
Collaborator

@SamuelCahyawijaya SamuelCahyawijaya left a comment

Choose a reason for hiding this comment

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

Thank you for adding the new schema @MJonibek. LGTM!

\\
NODE3 .....
"""
import datasets
Copy link
Collaborator

@sabilmakbar sabilmakbar Jan 7, 2024

Choose a reason for hiding this comment

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

Hi @MJonibek, would you like to add an example of this schema using one compounded sentence on the docstring? This should be similar to ToD Schema (#237), since this schema is quite complex and less used compared to others, which can hampers the implementation using this schema.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hi, ok I will do it today.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done. I tried to do small example, because all sentences from alt_burmese_tree_bank dataset are big. I think now it is clear how to use this schema

Copy link
Collaborator

Choose a reason for hiding this comment

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

Oh, so the dataset construction from the example sentence will be a top-down approach then, I thought it was a bottom-up. Looks fine to me, thanks a lot for proposing + implementing this, @MJonibek!

Copy link
Collaborator

@sabilmakbar sabilmakbar left a comment

Choose a reason for hiding this comment

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

LGTM! Let's wait for @SamuelCahyawijaya's review for this. Prior to merging, pls pull the changes first that has happened in master branch, @MJonibek.

\\
NODE3 .....
"""
import datasets
Copy link
Collaborator

Choose a reason for hiding this comment

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

Oh, so the dataset construction from the example sentence will be a top-down approach then, I thought it was a bottom-up. Looks fine to me, thanks a lot for proposing + implementing this, @MJonibek!

@SamuelCahyawijaya SamuelCahyawijaya merged commit 91623fe into SEACrowd:master Jan 9, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants