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

closes issue 5 #6

Merged
merged 8 commits into from Feb 21, 2020
Merged

closes issue 5 #6

merged 8 commits into from Feb 21, 2020

Conversation

arademaker
Copy link
Member

do not merge the complete history of this branch into MASTER. Only the last commit is relevant. As far as I know, using squash you will make all the commits in the branch into a single commit without preserving the intermediary files.

@arademaker arademaker changed the title Issue 5 closes issue 5 Feb 16, 2020
@huaiyu-zhu huaiyu-zhu self-assigned this Feb 19, 2020
@huaiyu-zhu
Copy link
Member

To clarify this pull request:

@huaiyu-zhu huaiyu-zhu removed their assignment Feb 19, 2020
@huaiyu-zhu huaiyu-zhu self-requested a review February 19, 2020 02:02
Copy link
Member

@huaiyu-zhu huaiyu-zhu left a comment

Choose a reason for hiding this comment

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

To be consistent with the convention used for other languages in this project, we need to change ARG0 to A0 and ARGM-TMP to AM-TMP, etc.

@arademaker
Copy link
Member Author

Hi @huaiyu-zhu , actually I believe it is the other way around. Following the same principle that @yunyaoli evokes to keep the current format (variable number of columns) similar to what Propbank uses. I believe we should also keep the same identifiers used by Propbank. We don't change the sense identifiers (e.g. "be.01"), so why do we changed the argument identifiers?

Propbank uses ARG0, ARGM-TMP, and ARGM-MOD (see http://verbs.colorado.edu/propbank/framesets-english-aliases/mantle.html). So I vote on the creation of a new issue to fix the other languages. I can handle it. As close as we follow the conventions and standards, the better will be.

Copy link
Member

@huaiyu-zhu huaiyu-zhu left a comment

Choose a reason for hiding this comment

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

Agree with keeping current arg label format of ARG*, and opening new issue to change from A* in other languages.

@arademaker arademaker merged commit 60e2fb8 into master Feb 21, 2020
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.

None yet

2 participants