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

Update some examples for AllenNLP 1.0.0 #5

Merged
merged 2 commits into from Jul 15, 2020

Conversation

mathcass
Copy link
Contributor

Contains a few fixes to update notebooks to AllenNLP 1.0.0.

Mostly, updates some of the data loaders to properly import from the new
allennlp-models package as well as fix up their instantiation and use.


I thought it might be helpful to update some of these to use AllenNLP >= 1.0.0. For most of them, I followed the example set in the updated sentiment notebook. I ran into a bit of trouble with the LM notebook where there the output data point seemed to be "wrapped" twice, nested as {"tokens": {"tokens": <vector>...

I figured I'd make a PR to help aid the process of transforming these. I'm also happy to close this if it's redundant.

Contains a few fixes to update notebooks to AllenNLP 1.0.0.

Mostly, updates some of the data loaders to properly import from the new
allennlp-models package as well as fix up their instantiation and use.

* [POS notebook output]
* [LM notebook output]
* [NER notebook output]
* [CNN notebook output]

[POS notebook output]: https://www.dropbox.com/s/svt2vsbwxr1m5if/pos_tagger.html?dl=0
[LM notebook output]: https://www.dropbox.com/s/6uu3g9v82475u67/lm.html?dl=0
[NER notebook output]: https://www.dropbox.com/s/n7kdvt652yceb43/ner.html?dl=0
[CNN notebook output]: https://www.dropbox.com/s/zir11ne7vbg7ark/sst_cnn_classifier.html?dl=0
Remove a few instances where I commented out code rather than just removing it.
Also, reorder some imports.
" Thus, calling output_tokens = output_tokens[\"tokens\"] to unnest `tokens`\n",
" resolves this in an unideal way.\n",
" \"\"\"\n",
" output_tokens = output_tokens[\"tokens\"]\n",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I couldn't quite figure out the correct way to wrap the data loaders from AllenNLP to avoid this nesting of the output_tokens.

@mhagiwara
Copy link
Owner

@mathcass Thanks for the PR!

@mhagiwara mhagiwara merged commit 693029b into mhagiwara:master Jul 15, 2020
@mathcass
Copy link
Contributor Author

Of course! If you figure out the issue above with nested "tokens" keys in the LM notebook, I'd love to know how to fix it!

@mathcass mathcass deleted the cp/pos-tagger branch July 15, 2020 16:57
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.

None yet

2 participants