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

tag future tense verbs #127

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open

tag future tense verbs #127

wants to merge 9 commits into from

Conversation

ThijsVroegh
Copy link
Collaborator

This pull request introduces the ability to tag future tense verbs in the action analysis module. It includes updates to both the backend logic for tagging verbs and the user interface to support this new feature. This solves issue #93

Backend changes:

  • Added support for tagging future tense verbs in the __postag_sents and postag_text methods by introducing a new future_vbz parameter.
  • Added the future_verb column to the complete_data_columns list in the Tagger class.
  • Implemented the __process_dutch_future_verbs method to identify future tense verbs in Dutch.
  • Updated the __parse_tagged_story method to include future tense verbs in the story dataframe.

UI changes:

  • Added a checkbox for "Actions - future tense" in the OWSNActionAnalysis widget.
  • Updated the show_docs method to pass the future_vbz parameter.
  • Added a new HTML mark style for future tense actions.

# Loop through each token in the sentence
for tok in tags:
lemma_value = tok[10]
text_value = tok[5]
Copy link
Member

@eriktks eriktks Nov 14, 2024

Choose a reason for hiding this comment

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

tok[5] contains the story_navigator_tag, not the token text. Use in tok[2] instead

Copy link
Member

@eriktks eriktks left a comment

Choose a reason for hiding this comment

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

I have tested the code and examined the changes. In the changes tok[5] should be replaced by tok[2] to get the text_value. Otherwise the code changes look fine. The tests were mostly fine. There was one exception: in "zullen en willen vergeten" where "willen" was tagged as future verb. This could have been avoided by checking the dependency relations but this is something which can be done later. Thanks for this work!

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.

2 participants