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
DM-21013: Add description for daf_butler expression parser #185
Conversation
02f7da5
to
e8a70b5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! Most comments are just minor grammar/typos issues.
doc/lsst.daf.butler/exprParser.rst
Outdated
Butler expression language | ||
========================== | ||
|
||
Butler registry supports user-supplied expression for constraining input, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
" supports a user-supplied"
doc/lsst.daf.butler/exprParser.rst
Outdated
QuantumGraph. This page describes the structure and syntax of the expression | ||
language. | ||
|
||
Language grammar is defined in ``exprParser.parserYacc`` module which is |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"The language grammar ... module, which is"
doc/lsst.daf.butler/exprParser.rst
Outdated
responsible for transforming string with user expression into a syntax tree | ||
with nodes represented by various classes defined in ``exprParser.exprTree`` | ||
module. Modules in ``exprParser`` package are considered butler/registry | ||
implementation detail and are not exposed at the butler package level. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"detail" -> "details"
doc/lsst.daf.butler/exprParser.rst
Outdated
possible errors. The language for expression was at that point decided to be | ||
a small subset of the expressions allowed in SQL WHERE clause. Later we | ||
started generating SQL code from a parsed syntax tree and extended the | ||
language with few additional constructs that are not in SQL. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure the history is relevant for a user guide; I think I'd just say that the language is similar to a subset of SQL with some extensions.
-------------------- | ||
|
||
The expression is passed as a string to a registry methods that build a | ||
QuantumGraph typically from a command-line application such as ``pipetask``. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It probably doesn't make sense to change this yet, as I imagine this ticket will land before DM-17023, but when DM-17023 does land, we should remember to update this paragraph (and the very first one) to reflect the fact that the expression language is used in more than just QuantumGraph generation there.
doc/lsst.daf.butler/exprParser.rst
Outdated
|
||
Identifiers represent values external to a parser such as values stored in a | ||
database. Parser itself cannot define identifiers or their values, it is | ||
responsibility of translation layer (registry) to map identifiers into |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"The parser itself"..."values; it is the responsibility of the translation later"
doc/lsst.daf.butler/exprParser.rst
Outdated
Identifiers represent values external to a parser such as values stored in a | ||
database. Parser itself cannot define identifiers or their values, it is | ||
responsibility of translation layer (registry) to map identifiers into | ||
something sensible. Like in most programming languages identifier starts with |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"languages, an identifier"
doc/lsst.daf.butler/exprParser.rst
Outdated
where each item in the right hand side list is one of the supported literals. | ||
Unlike regular SQL IN operator the list cannot contain expressions, only | ||
literals. The extension to regular SQL IN is that literals can be range | ||
literals as defined above. And it can also be a mixture of integer literals |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"above. It can also be"
doc/lsst.daf.butler/exprParser.rst
Outdated
and range literals (language allows mixing of string literals and ranges but | ||
it may not make sense when translated to SQL). | ||
|
||
For an example of range uses these two expressions are equivalent:: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"range usage, these two"
|
||
Language supports set of regular comparison operators - ``=``, ``!=``, ``<``, | ||
``<=``, ``>``, ``>=``. This can be used on operands that evaluate to a numeric | ||
values, for (in)equality operators operands can also be boolean expressions. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe add a note that equality comparison is a single =
, like SQL but unlike Python?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added this:
.. note :: The equality comparison operator is a single ``=`` like in SQL, not
double ``==`` like in Python or C++.
e8a70b5
to
1a10e32
Compare
1a10e32
to
be6a0c6
Compare
No description provided.