-
Notifications
You must be signed in to change notification settings - Fork 20
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
Move nltk download to inside the gender augmentation function #93
Conversation
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.
LGTM for the changes, let me suggest one more thing for this PR:
Found that actually the right patyh for nltk.data.find
and nltk.download
are different. That's why the code always takes except LookupError
and shows the nltk
log like that.
[nltk_data] Downloading package averaged_perceptron_tagger to
[nltk_data] /home/vscode/nltk_data...
[nltk_data] Package averaged_perceptron_tagger is already up-to-
[nltk_data] date!
[nltk_data] Downloading package punkt to /home/vscode/nltk_data...
[nltk_data] Package punkt is already up-to-date!```
[nltk_data] Downloading package averaged_perceptron_tagger to
[nltk_data] /home/vscode/nltk_data...
[nltk_data] Package averaged_perceptron_tagger is already up-to-
[nltk_data] date!
[nltk_data] Downloading package punkt to /home/vscode/nltk_data...
[nltk_data] Package punkt is already up-to-date!
Could you double-check that my two suggestions resolve the problem?
Co-authored-by: Koki Ryu <[email protected]>
Co-authored-by: Koki Ryu <[email protected]>
ohhh yeah good find!! We were doing this properly for stats and factual consistency 😅 Checked that your suggestions work |
This ensures that
nltk.download
will only run whenaugment.gender
is actually run, rather than whenimport langcheck
is run.