search

fangfangli / cleartk

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

Represent tree kernel features with types instead of naming conventions #366

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago 
The current way of indicating a tree kernel feature is to use the prefix "TK" 
in the feature name. Besides this only being documented in various comments, it 
doesn't seem like a good idea. Maybe it would be better to have a 
TreeKernelFeature type, even if all it does is take care of prefixing the 
feature name with "TK" so the tree kernel data writers know what to do with it.

Original issue reported on code.google.com by tim.mil...@gmail.com on 3 May 2013 at 11:16

GoogleCodeExporter commented 9 years ago 
Yeah, I agree that it would be better to have a TreeFeature subclass of 
Feature, and TreeFeatureVectorFeaturesEncoder could simply check for that with 
instanceof. Classifiers that don't use tree kernels would just use the regular 
string value of the feature (like they do now).

Original comment by steven.b...@gmail.com on 3 May 2013 at 11:45

GoogleCodeExporter commented 9 years ago 
Here is a patch implementing this fix.

Original comment by tim.mil...@gmail.com on 11 May 2013 at 7:12

Attachments:

GoogleCodeExporter commented 9 years ago 
The patch looks good. It needs a unit test though. (We try to add a unit test 
with every commit.)

Original comment by steven.b...@gmail.com on 12 May 2013 at 1:43

GoogleCodeExporter commented 9 years ago 
This issue was closed by revision 18c46b6303b1.

Original comment by tim.mil...@gmail.com on 13 May 2013 at 1:41