Fix 2183 Update QB Relationship Indicators #2224
Merged
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
|
I will have a look at it tomorrow. I can also show you then how to interactively rebase, so you can squash commits into nice commits, without losing the proper attribution to each one of you |
|
Coverage decreased (-7.1%) to 61.71% when pulling d1b68df4734100d1fd984a8194e3a09e2c0df414 on ConradJohnston:Fix_2183_Update_QB_Relationship_Indicators into aa79072 on aiidateam:provenance_redesign. |
1 similar comment
|
Coverage decreased (-7.1%) to 61.71% when pulling d1b68df4734100d1fd984a8194e3a09e2c0df414 on ConradJohnston:Fix_2183_Update_QB_Relationship_Indicators into aa79072 on aiidateam:provenance_redesign. |
ConradJohnston
force-pushed
the
Fix_2183_Update_QB_Relationship_Indicators
branch
from
c5a796d to
c3ec654
Compare
Also did some fixes to make sure that warnings are not suppressed by the re-configuration of the loggers by Django. Adding trajectory to prospector, and replacing NotImplementedError with FeatureNotAvailable or other custom exceptions where appropriate to avoid that propsector complains that it is a abstract method
Renames the querybuilder relationship names used for joins to the scheme.
Previously, complex names where used, but these were hard to remember. The new
scheme uses simple generic names, e.g. "with_computer" and gets the required
context from the class which is specified in the qb.append() method.
The main changes are:
1. - Change the relationship function map to a two-level dictionary, where the first
level provides the context, indicating what class of node the relationship applies to.
2. - Add deprecation messages which are printed when the join functions are called
using the old relationship names
3. - Remove the unimplemented "join_slaves/join_masters" methods
In addition, moving all deprecation warnings to the new system
(AiidaDeprecationWarning) and removing some very old deprecations
ConradJohnston
force-pushed
the
Fix_2183_Update_QB_Relationship_Indicators
branch
from
c3ec654 to
61b9126
Compare
ConradJohnston
deleted the
Fix_2183_Update_QB_Relationship_Indicators
branch
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 #2183 .
Changes the existing QueryBuilder join types from
input_ofandoutput_oftowith_outgoingandwith_incomingrespectively. For now,ancestor_of,descendant_ofremain in use. The remaining join types are changed towith_{entity}where the type of join performed depends contextually on the class passed in the qb.append() method. For example:QueryBuilder().append(User, tag='u').append(Group, with_user='u').all()Throughout the code, the old relationships have been replaced.
The old methods, if used, now print a deprecation warning, provided by the new
AiidaDeprecationWarningclass, fromaiida.common.warnings. This warning has the advantage of working with pycharm and not being swallowed likeDeprecationWarning. All deprecation warnings in the code have now been replaced by this new warning and the documentation has been updated.