Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Code for query expansion. #24

Closed

Conversation

aarshkshah1992
Copy link

No description provided.

* the name of the scheme to be used. If no scheme is specified,
* "bo1" is used.
*/
void set_expansion_scheme(const std::string eweightname_ = "bo1");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Either this should match the docstring or the docstring should match this.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please can you explain what this means ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@param eweight_ is not the name of the actual parameter.

// so we use "<=" not "<" here.
/* Set up the EXpandweight by clearing the existing statistics and
collecting statistics for the new term. */
(eweight.stats).clear_stats();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the () look leftover from a previous iteration

@@ -53,6 +53,8 @@
using namespace std;

using Xapian::Internal::ExpandWeight;
using Xapian::Internal::Bo1Eweight;
using Xapian::Internal::TradEweight;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The 'w' in 'weight' should really be capitalised in these class names (it's "trad. e. weight", not "trad eweight").

*
* @param eweightname_ A three character string in lowercase
* specifying the name of the scheme to be used.
* The following schemes are currently availabe:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"available"

double expand_k;

public:
public:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

two spaces

double k, const ExpandDecider * edecider, double min_wt) const
{
LOGCALL(API, Xapian::ESet, "Xapian::Enquire::get_eset",
maxitems | rset | k | flags | edecider | min_wt);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool, that looks a suitable compatibility wrapper.

We don't bother wrapping LOGCALL lines - they aren't aimed at human readers (since they just duplicate information from the function prototype). Ideally they'd just get added/updated automatically, but nobody's written a script to automate that task yet.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh okay, didnt know that. I am not undoing this change but will keep this in mind :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please actually make the change.

{
LOGCALL(MATCH, double, "ExpandWeight::get_weight", merger | term);
LOGCALL(MATCH, void, "ExpandWeight::collect_stats", merger | term);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LOGCALL_VOID() should be used in such cases - using void in the normal LOGCALL macro results in typedef void foo; when compiling with logging enabled, and some compilers don't like that.

@ojwb
Copy link
Contributor

ojwb commented Sep 27, 2013

Merged via SVN.

@ojwb ojwb closed this Sep 27, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants