GazetteerViewGenerator loads no gazetteers; deprecate in favor of SimpleGazetteerAnnotator?

[mssammon]

GazetteerViewGenerator and it is trying to load gazetteer files from resources/gazetteers/gazetteers, but this doesn't exist. Instead, I think clients should use SimpleGazetteerAnnotator, which loads resources from illinois-common-resources (note that these are organized differently from the way expected by GazetteerViewGenerator).

[shyamupa]

Hey @mssammon,
Is the resource organization the only issue? I see that the SimpleAnnotator takes a resource manager, so path etc. can be easily dictated. As far as I understand the only change in functionality between the two is that the old view generator uses some filters (POS, NP) that I did not find in the new SimpleAnnotator. That might be a problem, depending on what application uses this class.

[mssammon]

@shyamupa good to know, thanks. I think that ideally the option to use filters should be implemented in the new annotator -- I'm presuming this is non-trivial additional work but doable. Sound OK?

[cowchipkid]

The SimpleGazetteerAnnotator is highly optimized. It can do phrases of any length without impacting performance. The GazViewGen is based on the older methodology which is less efficient, but requires less memory. It is a simple hash set of all the phrased from the gaz, but for multi-token hits it is not very efficient. There are lots of cool features in GazViewGen. I suggest, if they are useful features, we should move functionality over to the SimpleGazetteerAnnotator, and deprecate the older GazViewGenerator.

[mssammon]

@shyamupa please track any usages of the old gazetteer view generator within illinois-cogcomp-nlp. (I assume none, but just in case...). What is the cost of migrating the GazViewGen features to the SimpleGazetteerGenerator?

[shyamupa]

@mssammon So the class is not used outside of KnownFexes,CreateTestFeaturesResource and TestWordFeatureFactory. We can migrate to the new class with almost no effect outside. I found that the POSFilter etc. filters that I was suspecting to cause problems are not used directly for any application inside the package. I think we can go ahead and deprecate the old class.

[shyamupa]

Should be taken care of by #244. Will close this issue once merged.