Various features and fixes related to Group subclassing and Group in the CLI
#3926
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov Report
@@ Coverage Diff @@
## develop #3926 +/- ##
===========================================
+ Coverage 78.22% 78.23% +0.01%
===========================================
Files 461 461
Lines 34050 34073 +23
===========================================
+ Hits 26634 26656 +22
- Misses 7416 7417 +1
Continue to review full report at Codecov.
|
sphuber
force-pushed
the
feature/3925/group-param-type-subclassing
branch
from
0ae0f86
to
0d79091
Compare
The recent addition of support for subclassing of `Group` entities to
the `QueryBuilder` introduced a bug when joining a tuple of entities to
an appended `Group` clause, e.g.:
builder = QueryBuilder().append(Group, tag='group)
builder.append((orm.Float, orm.Int), with_group='group')
This would cause an exception in `QueryBuilder._get_connecting_node`
that would assume that `self._path[index]['entity_type']` would always
be a string, but it can be a tuple as well.
sphuber
force-pushed
the
feature/3925/group-param-type-subclassing
branch
from
0d79091
to
b925885
Compare
|
@CasperWA thanks for the review. There is just two points open. I prefer to keep the unused variables just for consistency's sake, is that ok? And your first suggestion I did not understand. Not sure which option you were suggesting of being generalized. |
See my comments where applicable above. |
sphuber
force-pushed
the
feature/3925/group-param-type-subclassing
branch
2 times, most recently
from
34980b7
to
57cacbe
Compare
The `--group-type` option in `verdi group list` and `verdi group path ls` has been deprecated and superseded with `-T/--type-string`. The recently added support for subclassing of the `Group` ORM class has made it interesting to be able to query for specific subclasses based on the type strings of groups. Since these can be hierarchical it is useful to support the inclusion of SQL wildcards `%` and `_` to perform like instead of exact matching. Note that this new functionality of filtering is only available for `verdi group list` and not for `verdi group path ls`, as the `GroupPath` utility does not support subclassing yet.
This will allow options and arguments that are supposed to match `Group`
instance, to narrow the scope of subclasses that are to be matched.
For example, an option or argument with the following type:
GroupParamType(sub_classes=('aiida.groups:core.auto',))
will only match group instances that are a subclass of `AutoGroup`. Any
other `Group` subclasses will not be matched.
This will enable auto-completion for existing `Code` and `Group` instances by their label for commands that have options or arguments with the corresponding parameter type.
sphuber
force-pushed
the
feature/3925/group-param-type-subclassing
branch
from
57cacbe
to
35ce7b8
Compare
|
@CasperWA I addressed your final comment |
CasperWA
approved these changes
Apr 15, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #3925
This PR comprises four commits:
QueryBuilderthat was introduced recently by addedGroupsubclassing support--group-typefor--type-stringwhich provides querying with subclassingGroupParamTypeCLI typeCodeandGroupinstance options and arguments