-
Notifications
You must be signed in to change notification settings - Fork 8
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
Feature/corpus analysis #140
base: develop
Are you sure you want to change the base?
Conversation
This adds a fixture for a corpus freqdist, which generates a warning (since django tries to load a fixture for something that is not a model). For now I'll leave this warning. But maybe we should fix this before merging this branch.
We were getting some errors when tests ran because the write operation didn't finish before trying to retrieve data.
The view will also be used to run the analyisis, but is not yet working for that.
This also includes a change to the corpus analysis fixture, to keep the data with the "user" user.
This is what the backend expects.
@@ -44,6 +44,10 @@ def create_indexing_pipeline(doc): | |||
{"index_name": doc.index_name, "doc_type": doc.doc_type}}) | |||
(Extractor().si(doc_id) | ElasticIndexer().si(doc_id))() | |||
|
|||
def corpus_freqdist(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.
Shouldn't this have a verb-name, like calculate_corpus_freqdist
? I thought the call on the view was strange because of this.
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.
Agreed. I just pushed that change.
This adds the functionality in the frontend to use the functionality added in NAMD/pypln.backend#183 .
The first worker available calculates the freqdist for a worker.