(I must have messed up something with my github account -- now I get
"403 Forbidden -- Request forbidden by administrative rules"
seemingly everywhere. In the meantime, hope "reply directly" is a way
of answering that works)



Thanks, I'll commit a change of the assert.

However, note that this is not new code -- the code under
nltk.tag.brill.nltk2
is basically the previous version of nltk.tag.brill (sorry, should
have pointed that out more clearly in the pull request). Except the basic
splitting into
subpackages for clarity (and some automatized global changes to
header and docstring format),  I haven't changed anything.

All previous functionality (and more) is available in the proposal, but
the APIs aren't completely compatible. Still, the changes at the
callers' side are small, and maybe we should have a more general
discussion whether it is worth the maintenance effort to keep
the old nltk.tag.brill.nltk2 in the first place, before anyone puts
more effort into reviewing it. Thoughts?

Marcus




2013/11/19 Mikhail Korobov <notifications@github.com>

> In nltk/tag/brill/nltk2/template.py:
>
> > +    @staticmethod
> > +    def extract_property(token):
> > +        """
> > +        Returns some property characterizing this token, such as its
> > +        base lexical item or its tag.
> > +
> > +        Each implentation of this method should correspond to an
> > +        implementation of the method with the same name in a subclass
> > +        of ``ProximateTokensTemplate``.
> > +
> > +        :param token: The token
> > +        :type token: tuple(str, str)
> > +        :return: The property
> > +        :rtype: any
> > +        """
> > +        assert False, "ProximateTokenRules must define extract_property()"
>
> It is better to raise NotImplementedError here because assert will become
> no-op if python is executed with -O option.
>
> —
> Reply to this email directly or view it on GitHub<https://github.com/nltk/nltk/pull/549/files#r7746329>
> .
>