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 tense transform #135

Merged
merged 3 commits into from Oct 2, 2021
Merged

add tense transform #135

merged 3 commits into from Oct 2, 2021

Conversation

MukundVarmaT
Copy link
Contributor

This transformation converts sentences from one tense to the other, example: simple present to simple past.

@MukundVarmaT MukundVarmaT changed the title add tense tense transform add tense transform Jul 16, 2021
@kaustubhdhole kaustubhdhole self-requested a review July 27, 2021 11:52
super().__init__()
assert to_tense in ['past', 'present', 'future', 'random']
self.to_tense = to_tense
self.nlp = spacy.load('en_core_web_sm')
Copy link
Collaborator

Choose a reason for hiding this comment

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

from initialize import spacy_nlp
self.spacy_model = spacy_nlp if spacy_nlp else spacy.load("en_core_web_sm")

doc = self.nlp(text)

out = list()
out.append(doc[0].text)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why not change the tense of all sentences?

Copy link
Contributor

Choose a reason for hiding this comment

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

There seems to be a misunderstanding , we are indeed changing the tense of all sentences, here the variable out is just initialised with the first word(token) in the given sentence, a few lines below for word in doc we iterate over all words in the sentence

return newWord

'''
change tense function borrowed from https://github.com/bendichter/tenseflow/blob/master/tenseflow/change_tense.py
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please add this in the README too.

"to_tense": "past"
},
"inputs": {
"sentence": "I will go to the park."
Copy link
Collaborator

Choose a reason for hiding this comment

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

Since the transformation contains a lot of logic, can you also add longer sentences too (containing more than 1-verb) and also add cases where the transformation works great and where it does not.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for pointing this out , we have added a few more test cases to help provide a better picture

tasks = [
TaskType.TEXT_CLASSIFICATION,
TaskType.TEXT_TO_TEXT_GENERATION,
TaskType.TEXT_TAGGING,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Tagging would not be applicable since the number of tokens would change.


def generate(self, sentence: str):
perturbed_texts = self.change_tense(sentence, to_tense = random.choice(['past', 'present', 'future']) if self.to_tense == 'random' else self.to_tense)
return [perturbed_texts]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please add a newline.

@kaustubhdhole
Copy link
Collaborator

This looks fine overall. Please address the comments! Thank you very much for your efforts!

@tanay2001
Copy link
Contributor

This looks fine overall. Please address the comments! Thank you very much for your efforts!

Thanks for your review @kaustubhdhole
We have addressed the concerns hope its fine now !

}
```
### Data and Source Code
change tense and verb infliction borrowed from https://github.com/bendichter/tenseflow
Copy link
Collaborator

Choose a reason for hiding this comment

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

Thank you @tanay2001! Since some of the code was derived from this work, I suggest inviting the authors of this repository to be included as the authors of this transformations. Have you reached out to them?

@kaustubhdhole
Copy link
Collaborator

Hi @tanay2001, thanks for addressing the concerns. I think this is good to go. Just one minor thing - please reach out to the authors of the repo once to invite them :) Thanks again for your changes!

@kaustubhdhole kaustubhdhole merged commit 736fa20 into GEM-benchmark:main Oct 2, 2021
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

3 participants