Anushkasandaruwan / cleartk

Automatically exported from code.google.com/p/cleartk
0 stars 0 forks source link

Make CleartkExtractor.Ngrams produce ContextFeature #375

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
In order to use aggregating contexts with Ngrams, it needs to create 
ContextFeatures. Please see attached patch.

Original issue reported on code.google.com by alexey.v...@gmail.com on 7 Jun 2013 at 8:32

Attachments:

GoogleCodeExporter commented 9 years ago

Original comment by steven.b...@gmail.com on 7 Jun 2013 at 2:23

GoogleCodeExporter commented 9 years ago
we decided to extend this patch a bit to make sure that we always create 
ContextFeatures in e.g. Ngram, Ngrams, Bag, and Count.  It may be convoluted to 
e.g. Count and Ngram, but we came up with some hypothetical scenarios in which 
this sort of thing ought to work.  

Original comment by phi...@ogren.info on 23 Jul 2013 at 6:01

GoogleCodeExporter commented 9 years ago
This issue is fixed in rev a3a41e8cce62f6d8a63e1e76679fbba38f80f0bf

Alexey, thanks again for the patch.  It was a simple and straightforward fix.

I'm not perfectly happy with how we extended this to make other scenarios work 
and the code got a bit complicated.  It probably doesn't work as it should for 
certain nestings of contexts.  However, all the tests pass and it should allow 
for more flexible nesting of contexts including the scenario Alexey cares 
about.  If it turns out that there is some configuration that someone comes up 
with and it doesn't work, then they will just have to file a bug.  For example, 
I tried to nest a count inside a bag inside a count and I don't think it 
worked.  But my brain hurts trying to figure out what the feature should 
actually look like and so I'm not going to obsess over this anymore.  

Original comment by phi...@ogren.info on 4 Aug 2013 at 4:53