👌 IMPROVE: Add typing to QueryBuilder #5063
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #5063 +/- ##
===========================================
+ Coverage 80.43% 80.47% +0.05%
===========================================
Files 530 530
Lines 36946 36944 -2
===========================================
+ Hits 29712 29727 +15
+ Misses 7234 7217 -17
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
chrisjsewell
force-pushed
the
type-querybuilder
branch
from
b5e1dcf to
b347a30
Compare
chrisjsewell
force-pushed
the
type-querybuilder
branch
from
7c391e7 to
1a14258
Compare
In preparation for migration to sqlalchemy 1.4
chrisjsewell
force-pushed
the
type-querybuilder
branch
from
1a14258 to
f39c192
Compare
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
aiida/orm/querybuilder.py
Outdated
| NODE_CLS_TYPE = Type[Any] # pylint: disable=invalid-name | ||
| PROJECT_TYPE = Union[str, dict, Sequence[Union[str, dict]]] # pylint: disable=invalid-name | ||
| FILTER_TYPE = Dict[str, Any] # pylint: disable=invalid-name | ||
| ROW_TYPE = Any # pylint: disable=invalid-name |
There was a problem hiding this comment.
Why use here all-caps, but the CamelCase for PathItemType further down? I see most built in types having CamelCase, so is that the standard? Should we change these to have the same style, even though they are "constants" and normally would be all-caps?
There was a problem hiding this comment.
Don't know really lol, I think I say it somewhere else and kind of liked it, to distinguish these type annotation variables from regular one. But its not a big deal, so fine to change
There was a problem hiding this comment.
I just find it confusing when reading the type annotations seeing PathItemType and then ROW_TYPE. Think if that is also RowType it would be clearer and finding where the definition comes from is easy
There was a problem hiding this comment.
no worries, changed it 👍
This PR principally adds typing to aiida/orm/querybuilder.py, and does a small bit of refactoring:
__init__andappendconstructor variables more explicit, as opposed to relying onkwargswhich obfuscates the allowed inputs for the userself._aliased_pathvariable, whose use in relation to thedirectionjoining keyword was actually removed in v0.8 (nowdirectionis converted towith_outgoingorwith_incomingbefore storage, and similarly its value is converted from an int to a tag string)autotag=Truefrom.appendcalls; another legacy bit of codeThe PR is intended as preparation for migration to sqlalchemy 1.4 (and dropping sqlalchemy-utils)