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 16:22:35 GMT
 Sorry for the previous reply, pressed send before writing :-( ...

On 9/8/2010 10:40 AM, Adam Lally wrote:
> On Fri, Sep 3, 2010 at 3:28 PM, Marshall Schor <msa@schor.com> wrote:
>
>> 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.
>>
>> I agree with doing the above - better documentation and error messages are
> always good.
>
>
>
>> 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.
>>
>>
> My thinking on this is that this adds additional complication that's not
> worth it.  Currently we follow a straightfoward and often-used convention:
>
> * The annotator interface defines the API between framework and the user's
> component
> * The abstract base class provides default implementations of the methods,
> as a convenience
>
> Your suggestion breaks this convention, basically by having the abstract
> base class expose additional functionality (like a setContext() method) that
> the framework knows about and calls.  In my experience things are not
> normally done that way, so developers may be confused by what is happening.
> It's kind of a "back door" way of accessing the annotator.
>

I agree it's kind of a back door, not normally done thing.
> I also do not think that calling super.initialize() is that unusual of a
> requirement when overriding a method.

We also would need to have any user who implements their own
setResultSpecification() implementation, call super.setResourceSpecification(...).
> I admit that the current annotator interface is not ideal.  If I had it to
> do over again, I would have the UimaContext be a parameter to the
> reconfigure method. But that is not really feasible at this point.  IMO, the
> proposed patches for this problem all seem to have more downsides (in added
> complexity) than upsides.

The added complexity seems completely hidden from the annotator writers unless
they are reading the source for the superclasses; we could name methods for
doing this something that indicates its a call that the framework uses, and is
not for user use (and also document in a javadoc about it).  And these framework
helper methods could be marked "final" to prevent users from accidentally
overriding them.

Is the complexity you're concerned about something affects UIMA Framework
developers or do you believe it would affect the UIMA Annotator developers?  My
sense is that most annotator developers don't read the super-class source code
at all; they just follow the quick-start instructions on the website, or the
tutorials, or someone else's skeleton implementation.

The documentation we have shows a call to super.initialize(...) in most
initialize(...) examples, but not always.  And it doesn't say it's required,
although theoretically it's not required if you don't extend from the provided,
built-in base classes.

The sandbox has several examples where the writer forgot or didn't know they
needed to call super.initialize(...).  We can of course fix those.

The javadocs for the initialize() interface/methods and the
setResultSpecification methods that are user-code overridable I think should
mention that other parts of the framework-supplied code depend on these being
called, so any overriding method should call the super.

I guess for me it's a trade off of writing more documentation that we hope users
read and follow, vs. having framework code (that uses a "back-door" approach)
that just makes things work (in the vast majority of actual use cases).

-Marshall
>  -Adam
>

Mime
View raw message