Omega: Missing support for newer weighting schemes

[Olly Betts]

On Fri, Apr 14, 2017 at 12:31:08AM +0530, Vivek Pal wrote:
> Thanks Olly for clarifying. So, we want to define a static parse_params
> method needs to defined be in Xapian::Weight that would take "scheme" and
> a Registry object to do the following set of things:
> 
> 1. get an appropriate weighting scheme object by looking at the first
> word in "scheme" string,
> 
> 2. call parse_params method defined in that particular weighting scheme
> subclass with just parameter string (e.g. parse_params("1.0 0.5")) which
> creates and returns the weighting scheme object.
> 
> 3. then ultimately return the weighting scheme object to the code in omega:
> 
>     enq.set_weighting_scheme(Xapian::Weight::parse_params(scheme));
> 
> This is my current understanding of the main idea here but if you see
> something odd then please let me know.

Yes, that's the idea.

On Fri, Apr 14, 2017 at 12:31:58AM +0530, Vivek Pal wrote:
> >> > enq.set_weighting_scheme(*wt->set_parameter_values(p));
> >>
> >> set_parameter_values is basically parse_params method; an attempt to
> >> find a better name for parse_params.
> >
> > It's not a setter either - that method name suggests it is setting the
> > values on wt, but it actually returns a new object.
> 
> I'm kind of running out of names for this method. Would you like to suggest
> one?

My best idea so far is "create" for the static method which takes the
full string and an optional Registry.

We probably don't want to use the same name for the virtual method which takes
just the parameter part, especially as both would be callable with a single
string (I did use "parse_params" to refer to both, but that was mostly because
I got mixed up as to which I was talking about).

Perhaps "create_from_parameters" for that one?  It's conceivably useful to
call from user code (if you want to fix the weighting scheme and just allow
the parameters to be specified) but the longer name is a bit of a signal
that it's less commonly used.

On Fri, Apr 14, 2017 at 12:32:26AM +0530, Vivek Pal wrote:
> > Currently omega has no test coverage for parsing of weighting schemes, so the
> > tests passing doesn't mean this works correctly.
> >
> > (We only added any tests of the omega CGI about 18 months ago, and at present
> > we really only have tests of changes since then - feature tests for new
> > features and regression tests for bug fixes).
> 
> Oh, didn't realise there were no tests for this functionality.
> Probably, the next thing would be to write some tests afterwards.

Yes, that'd be great.  

Cheers,
    Olly