-
Notifications
You must be signed in to change notification settings - Fork 2.2k
Conversation
@@ -47,7 +47,8 @@ def test_read_from_file(self): | |||
"VP(TO to)(VP(VB be)(ADJP(JJ fair)(PP(TO to)(NP(JJ other)(NNS " | |||
"bidders))))))))))))))(. .)))") | |||
|
|||
assert fields["gold_tree"].metadata == gold_tree | |||
assert fields["metadata"].metadata["gold_tree"] == gold_tree | |||
assert fields["metadata"].metadata["tokens"] == tokens |
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 always feels clunky to me, I wonder if MetadataField
should implement __getitem__
? (not in this PR of course, just in general)
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.
We could do this for all of our fields, e.g. for token in TextField
etc, etc. I'm a fan
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.
What does for token in TextField
return, and when is it called?
For the MetadataField
, it was originally a hack to get something working for BiDAF. There is definitely room to improve it.
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.
it would be equivalent to that list comprehension (or probably returning Tokens
). Maybe it's less useful because it's unclear what it would return, but it would make it much cleaner.
* 'working' constituency parsing demo * wire the actual sentence through model, predictor and dataset reader * 90% working hierplane vis * tidy up, remove sentence lengths from return dict * clean up some js, remove spans from the demo * better description of the model * add explicit hierplane tree in test
The demo looks like this:
(We've hit maximum text wrapping capacity on a mac on the demo - we're going to need a "Select a demo model" page pretty soon.... it could look like this but for deep learning models)