uima-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Marshall Schor <...@schor.com>
Subject Re: handling of context saving in annotators
Date Wed, 08 Sep 2010 03:29:22 GMT


On 9/6/2010 4:04 AM, Tommaso Teofili wrote:
> Hi all,
> I had a look at:
> http://uima.apache.org/doc-uima-annotator.html
> and the initialize() method does not appear to be overridden so the
> super.initialize() call is not mentioned.
>
> If we go for the option of better documenting the super.initialize() need,
> maybe a good choice would be to put it here :
> http://uima.apache.org/downloads/releaseDocs/2.3.0-incubating/docs/html/tutorials_and_users_guides/tutorials_and_users_guides.html#ugr.tug.aae.developing_annotator_code
>
> or here:
> http://uima.apache.org/downloads/releaseDocs/2.3.0-incubating/docs/html/tutorials_and_users_guides/tutorials_and_users_guides.html#ugr.tug.aae.accessing_parameter_values_from_annotator
>
> Despite this, my opinion is that having not to write the
> "super.initialize();" inside a method overriding initialize() would be much
> better.

I agree, because then we can avoid the error, rather than report it to the user
who may then need to contact the annotator writer (maybe a different person) and
have them correct their code.

I always like to avoid errors rather than report them, where feasible.

Also, I realize that there is another field that could cause similar problems
for annotator writers - the one that holds the result specification, and is
returned by the default implementation of getResultSpecification().

Our examples (e.g., the PersonTitleAnnotator) use this method.  If an annotator
write was to include his own setResultSpecification method, and neglect to call
the super for that, and then use the getResultSpecification() method, it would
return null.

So, both of these could be set by the framework, if the framework determines the
analysis component is extending one of the default super classes, and thus,
avoid these errors.

-Marshall
> I will take a look at other sandbox annotators and check for the
> super.initialize() issue.
Thanks.
> Regards,
> Tommaso
>
>
>
>
> 2010/9/3 Marshall Schor <msa@schor.com>
>
>>  A recent "bug" in the Snowball annotator arose as follows:
>>
>> 1) It didn't implement the full annotator set of methods, but instead,
>> depended
>> on the implementation skeleton done by a "base" implementation class.
>> 2) That base implementation class included an implementation of
>> reconfigure()
>> which is implemented as:
>>
>>  public void reconfigure() throws ResourceConfigurationException,
>> ResourceInitializationException {
>>    destroy();
>>    initialize(mContext);
>>  }
>>
>> where "mContext" is a field which might or might not be set (it isn't set
>> by the
>> framework).
>>
>> It is set by the base implementation class's implementation of the
>> "initialize(uimaContext)" method.  If the user has their own initialize
>> method,
>> and doesn't set this variable (which he would not normally be aware of)
>> himself,
>> he can set it by calling "super.initialize(uimaContext)".
>>
>> The bug that arose happened when the annotator writer didn't call
>> "super.initialize", and then the application writer (different people,
>> different
>> roles) called "reconfigure", which the framework eventually passed down to
>> the
>> annotator.
>>
>> The user didn't implement this, but instead depended on the base
>> implementation
>> class.  This referenced the null value for mContext, which caused the
>> error.
>>
>> ========
>>
>> There are different kinds of things one could do here.  One would be to add
>> some
>> more notes to the manual(s) (I didn't check, the notes might actually
>> already be
>> there, but we could in that case make them stand out more :-) ) saying that
>> if
>> you use the base implementation classes as superclasses of your annotator
>> implementation, you *must* call super.initialize.
>>
>> Another would be to add some support in the getContext() default method
>> which
>> checks for null, and if found, issues a longer error message stating that
>> the in
>> user's implementations of initialize which extend xyz base class, the user
>> *must* call super.initialize..., before calling getContext() or other
>> framework
>> methods that call getContext() (such as getEmptyCAS - used in CAS
>> multipliers),
>> or words to that effect.
>>
>> Another thing that could be done is to have the framework examine the
>> annotator
>> class before it calls its initialize method, to see if it is extending one
>> of
>> the base impl classes which has this field and makes use of it in the
>> getContext() method, and if it finds that to be true, the framework could
>> set
>> this field itself.  This would eliminate the possible error from happening,
>> but
>> maybe it's overkill for what is a simple thing for users to do.
>>
>> ========
>>
>> Are there other approaches to this, and which kind of fix do you think we
>> should
>> do for this?
>>
>> -Marshall
>>
>> P.S., the Snowball Annotator in our sandbox has been fixed
>> (https://issues.apache.org/jira/browse/UIMA-1861).  The other annotators
>> in our
>> sandbox and our examples haven't been "reviewed" as yet to see if they have
>> this
>> issue.
>>

Mime
View raw message