-
Notifications
You must be signed in to change notification settings - Fork 2
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
Tupelize corpus #32
Tupelize corpus #32
Conversation
Leave this be for the moment.. Related to the setting widget, s odepending on how we are going to develop the widget further, we can later decide on deleting this PR, or not |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR adds a useful function to the code and tests it. It is unfinished: the function is defined but not called. But the code is tested and the test function could be used as a blueprint for the other code.
The code and test work, and the code looks ok.
We do suggest to move the test file and test data to the standard test directory (tests) before merging
@@ -4,6 +4,8 @@ | |||
import sre_constants | |||
from typing import Any, Iterable, List, Set | |||
import numpy as np | |||
import pandas as pd |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pd is imported but not used
@@ -290,3 +291,18 @@ def find_verb_ancestor(token): | |||
|
|||
# If no verb ancestor found, return None | |||
return None | |||
|
|||
def tupelize_corpus(corpus: Corpus): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tupelize_corpus is defined but not used
This PR was included in PR #110 and was closed when that PR was merged |
Issue
I separated out the code from
orange-story-navigator/orangecontrib/storynavigation/widgets/OWSNTagger.py
Line 103 in 80d4cf1
to a separate function that can be reused by other widgets (among others, in #14 )
tests run with
@kodymoodley , if you agree, before merging I'll replace the relevant bit in the line cited above with this function, too.
Description of changes
Includes