-
Notifications
You must be signed in to change notification settings - Fork 6
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
Review the internal data structures in class Query #158
base: develop
Are you sure you want to change the base?
Conversation
in the ORDER BY and the WHERE clause respectively
It may be subject to discussion whether this is a breaking change that need to wait for a major release. One could argue that the documented API has only been augmented, the legacy use is still supported. But if any third party code was relying on the (intentionally not documented) internal data structures, for instance in subclasses of |
conditions argument. Not yet covered: tests, documentation examples.
argument to Query.addConditions()
argument when creating queries. Not yet done: verify that evaluating the formal string representation of Query works as expected, add tests for the legacy use of a mapping and verify that the deprecation warning is indeed being raised.
when creating queries - While we are at it: eliminate a few more occurences of the old legacy ICAT query syntax in the tests
operator of Query may indeed be used to create an equivalent query to the tests in test_06_query.py where it makes sense
argument in class Query. Also verify that a DeprecationWarning is raised.
argument when creating queries. Eliminate a few more occurrences of the old legacy ICAT query syntax on the way.
to use the new format of the conditions argument when creating queries.
IDS" to use the new format of the conditions argument when creating queries.
in the ORDER BY clause
order of the keyword arguments and the order of calling the methods during initialization. This should essentially only have an impact on the presentation of the module reference page in the documentation.
of the conditions argument is done in test_06_query.py
Review the internal data structures to represent the items in the
ORDER BY
and theWHERE
clause in classQuery
. Implement dedicated helper classes to represent these items. As a result of #89, these data structures used to be a mapping of attribute names to format strings to build the corresponding item or condition in theORDER BY
orWHERE
clause respectively. Now, we use lists of corresponding helper class objects instead. Retaining all the information used to build the item (attribute name, JPQL function if used, right hand side of the condition, direction in the order) in separate attributes of the helper class objects and building the format string on demand as needed makes the code more clear and consistent.As a byproduct, we fix the formal string representation operator of class
Query
and close #94. Furthermore, we drop the restriction that any attribute may only appear once in theORDER BY
clause, as there is not really a justification for that restriction. E.g., the following is now allowed:The changes are on the internal data sructures, there are only few changes in the documented API. Most prominently: the
conditions
argument toQuery.addConditions()
now accepts a list of tuples, with a pair of an attribute name and a condition on that attribute, respectively. The previous format, a mapping of attribute names to a (lists of) conditions, is still supported, but deprecated. E.g. you'd need to changeto
While we are at it, also eliminate a some occurences of the old legacy ICAT query syntax in the code.
Draft status. Still TODO:
conditions
argument,conditions
argument,test_06_query.py
module: update theconditions
argument as in the producion code, add tests to verify that evaluating the formal string representation operator ofQuery
may indeed be used to create an equivalent query, add tests for the legacy use of a mapping and verify that the deprecation warning is indeed being raised,conditions
argument,conditions
argument, in particular, update the tutorial.