convert CharacterCategoryPatternExtractor to a feature function

[bethard]

Original issue 390 created by ClearTK on 2013-10-27T17:12:31.000Z:

It seems like CharacterCategoryPatternExtractor ought to be a FeatureFunction. Is there any reason why it isn't? The argument for making it a FeatureFunction is so that feature extraction isn't repeated and so that we have a consistent pattern for feature extraction. Thoughts?

[Steve]
Agreed that it should be a feature function.

[bethard]

Comment #1 originally posted by ClearTK on 2013-10-29T04:04:22.000Z:

It occurs to me as I start making this change that it could easily be both a feature function and a feature extractor. Feature functions are nice if you are going to use FeatureFunctionExtractor along with a bunch of feature functions. However, it actually makes the timeml code a little more complicated because instead of adding a simple CharacterCategoryPatternExtractor to the list of extractors you have to create a FeatureFunctionExtractor and initialize it with a CoveredTextExtractor and the CharacterCategoryPatternFunction. It doesn't look that pretty really. So, I think it can be both. The question is what to call it (extractor or function)?

I suppose another possibility is to have a CharacterCategoryPatterUtility class (or some such) and have both a CharacterCategoryPatternExtractor and CharacterCategoryPatternFunction.

Thoughts?

[bethard]

Comment #2 originally posted by ClearTK on 2013-10-29T05:04:00.000Z:

Yeah, I would have both a CharacterCategoryPatternExtractor and a CharacterCategoryPatternFunction and just have them share code somehow, either a common superclass or a utility class as you suggested.

[bethard]

Comment #3 originally posted by ClearTK on 2013-10-29T05:11:17.000Z:

I guess one more alternative would be to have a static method CharacterCategoryPatternFunction.asExtractor() that just wraps CharacterCategoryPatternFunction in a FeatureFunctionExtractor. This might have the benefit that we could supply the same .asExtractor() static method on the other FeatureFunctions, making them easier to use without manually creating a FeatureFunctionExtractor.

[bethard]

Comment #4 originally posted by ClearTK on 2013-10-30T05:19:02.000Z:

I had exactly the same thought this morning before I saw this.

I'm not familiar with the as* naming convention. Did you just make that up?

[bethard]

Comment #5 originally posted by ClearTK on 2013-10-30T13:41:10.000Z:

I've seen it before, but not sure where. Might be a scala-ism. I'd be fine with getExtractor() or createExtractor() or something like that too.

[bethard]

Comment #6 originally posted by ClearTK on 2013-10-31T04:44:34.000Z:

This issue was closed by revision 517d7cdda9f4.

[bethard]

Comment #7 originally posted by ClearTK on 2013-10-31T04:45:16.000Z:

I added two createExtractor methods one that uses a default PatternType and another that takes one as a parameter. For backwards compatibility the returned feature extractor is of type NamedFeatureExtractor1 (since that is what this class implemented when it was a feature extractor.) Tests are all passing and code is checked in.