[email protected] wrote:
On Mon, Apr 13, 2009 at 02:43:04PM -0700, Brock Pytlik wrote:
[email protected] wrote:
client/query_parser.py:
- lines 41-43: This sentence is missing some words that would allow
it to make sense. Did you mean the following?
This class overrides the base QueryParser class. When building the
AST, the client QueryParser needs to apply a different approach than
its parent class.
I'm not sure what words are missing. It applies exactly the same
approach (algorithm), but uses different building blocks for the AST.
This is the original comment:
This class exists so that the classes the parent class query
parser uses to build the AST are the ones defined in this module
and not the parent class's module. This is done so that a
single query parser can be shared between the client and server
modules but will construct an AST using the appropriate classes.
I think I'm confused by the relationship between this class's existence
and the behavior of the parent class's query parser. There's definitely
a missing word in the first sentence. Is it the following?
This class exists so that the classes __that__ the parent class
query parser uses to build the AST are the ones defined in this
module and not the parent class's module.
I could also see 'of' in place of 'that', but that makes the rest of the
sentence make no sense. If the above is correct, perhaps it would be
more clear to say this:
This class overrides a variety of methods that are defined in
the parent class. These subclassed methods are how the client's
query parser builds its AST.
Or have I missed the point completely?
Let's meet and talk this out (I just stopped by your office but missed
you). I think we're having a fairly large miscommunication here :)
- lines 87 and 88: If this is where you handle the search for
specific query terms, where do you handle the search for general
query terms?
Is 'particular query term' better? 'base query term'? A query is made up
of several query terms, which themselves can be made of query terms. I'm
trying to express that this is the lowest level of query terms. These
terms are not made up of other terms.
I think either of these would work well.
Ok
indexer.py:
- line 114: make "action type" consistent with whichever choice you
make above.
This I don't understand. I made it "These dictionaries", I don't believe
further modification to "action type" is necessary.
Perhaps it isn't necessary, but it would be clearer to refine this
further. I didn't completely understand what this meant until I read
the response from you. How about:
The action type and key indexes, which are necessary for
efficient searches by type or key, store their file handles in
dictionaries. File handles for actions are in at_fh, while
filehandles for keys are kept in st_fh.
Sure
- line 347: Is it possible to have a list that's not of type list?
Would you prefer arguments in place of list? I was trying to say, it
takes lists, and more than that, it takes lists with a specific type.
I really wasn't trying to be rhetorical here. I was trying to figure
out if you had imposed a specific type requirement on your lists -- by
subclassing maybe? However, it sounds like you were trying to explain
that the function expects a list with a pre-determined format of its
contents. If the latter is true, consider stating this in a way that
expresses that the lists must be in a format that the callee expects.
Perahps I'm being obtuse, or pedantic, but type implied class/instance
identity, which isn't what you mean.
Heh, didn't think you were being rhetorical. Types != class/instance for
me at least. That's why I put the type signature to describe the type
expected. You're right, it does expect lists that meet a certain
type/format. I thought I was conveying that the function only accepts
two lists of that type by saying that it takes two lists of a specific
type. I'm open to suggestions on how to reword this bit but since I'm
having trouble understanding the reading you took from it, I'm not
exactly sure how to reword it.
Is this clearer?
"""Takes two arguments. Each of the arguments must be lists with the
type signature ('a, list of 'b).
- line 300: Parse the string and input into ...
No, input is the variable name. So I do mean "Parse the string, input,
into an AST..."
Okay. It might be worthwhile to change this to "the string, named
input, into..." That way you'll avoid a miscue by your less astute
readers. (Me)
:)
- line 316: Singlular / plural disagreement here. Writing
this as the following might help:
Text strings are the tokens and syntax of the query.
Text is the variable name. I don't believe there is a disagreement here.
In that case, perhaps it would be better as, "Text is the variable that
contains the toakens and syntax of the query."
Yeah, all of these are going to be reworked as part of the
capitalization issue.
- line 458-460: I don't understand what this means. Can you explain
it a different way?
I can remove it, or I can have a fairly long explanation which would
roughly be this:
Restriction is only set by boolean queries, and currently only by AND
queries. Further, if this node is returning packages, then the query
containing it would also contain packages, and therefore not make use of
restriction (hence it would be none). Therefore, the only way
restriction is not None is when this query is contained within an AND
query higher up in the AST tree.
Since you've gone to the trouble of writing it here, there's no reason
why we couldn't modify the docstring. How about: (starting at 457)
The variable 'restriction' is a generator of actions, over which
the search shall be performed. Only boolean queries will set
restriction. Queries that are returning packages do not set
restriction. Nodes that return packages have, by definition,
parents that must return packages. This means that only queries
contained within a boolean query higher up in the AST tree will
have restriction set.
Or did I misunderstand how this works?
Yep, that's right (modulo one nitpick). I wasn't sure that belonged in a
docstring, but I'm happy to add it.
Nitpick: queries which return packages could contain nodes which set
restriction: <foo AND bar> is an example. This may change someday when
query optimization gets worked in well.
-j
Thanks,
brock
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
