-
-
Notifications
You must be signed in to change notification settings - Fork 252
Two features: allowing custom operators and dynaimc ignoring order #252
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
Conversation
Minor cleanup to documentation wording regarding iterable_compare_func
Add ignore_order_func to make ignore-order operation dynamic with level
allow custom operators to do/report some custom operations allow
hi @seperman would you like to do a CR for there two features?~ |
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.
Hi @eggachecat
Thanks for this PR. These are great new features! 👍
I had a couple of minor comments. Please take a look at my comments when you have a chance.
Also do you mind adding the documentation for the 2 features if you have time? It will be more authentic if the docs are written by you. If I write the docs, it will be my own interpretation of what these features mean. Looking at the code seems like you actually had a use case for these features.
The ignore_order_func
docs can go to ignore_order.rst
and the custom_operators
docs can go to other.rst
.
deepdiff/diff.py
Outdated
# compatibility | ||
if "ignore_order_func" not in _parameters: | ||
_parameters["ignore_order_func"] = lambda *_args, **_kwargs: _parameters["ignore_order_func"] | ||
|
||
if "custom_operators" not in _parameters: | ||
_parameters["custom_operators"] = [] | ||
|
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.
You don't need these for the compatibility. The _parameters
is a self.__dict__
copy that is passed. If you remove these lines, the only test that fails is one delta test since it has _parameters
hard-coded. Pleased update that test instead: https://github.com/seperman/deepdiff/blob/master/tests/test_delta.py#L1055
deepdiff/diff.py
Outdated
self.ignore_order = ignore_order | ||
|
||
if ignore_order_func is not None: | ||
self.ignore_order_func = ignore_order_func |
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.
You could use the same simple line like the one for the custom_operators: self.ignore_order_func = ignore_order_func or lambda *_args, **_kwargs: ignore_order
deepdiff/diff.py
Outdated
|
||
if not self._skip_this(level): | ||
level.report_type = report_type | ||
level.additional[CUSTOM_FILED] = extra_info |
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.
Typo: CUSTOM_FIELD
iterable_compare_func: | ||
:ref:`iterable_compare_func_label`: | ||
There are times that we want to guide DeepDiff as to what items to compare with other items. In such cases we can pass a iterable_compare_func that takes a function pointer to compare two items. It function takes two parameters and should return True if it is a match, False if it is not a match or raise CannotCompare if it is unable to compare the two. | ||
There are times that we want to guide DeepDiff as to what items to compare with other items. In such cases we can pass a iterable_compare_func that takes a function pointer to compare two items. The function takes three parameters (x, y, level) and should return True if it is a match, False if it is not a match or raise CannotCompare if it is unable to compare the two. |
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.
Thanks for fixing this. I thought I had found all the instances of this incorrect sentence already but obviously not!
@@ -0,0 +1,141 @@ | |||
import math |
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.
Thanks for adding the thorough tests!
deepdiff/model.py
Outdated
path = change.path(force=FORCE_DEFAULT) | ||
self['repetition_change'][path] = RemapDict(change.additional[ | ||
'repetition']) | ||
'repetition']) |
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.
This line is giving pep8 error E126.
okay! I will make fix these soon |
@seperman just updated the PR~ |
Codecov Report
@@ Coverage Diff @@
## dev #252 +/- ##
==========================================
- Coverage 99.63% 99.62% -0.02%
==========================================
Files 13 14 +1
Lines 2768 2900 +132
==========================================
+ Hits 2758 2889 +131
- Misses 10 11 +1
Continue to review full report at Codecov.
|
Hi @eggachecat |
Hi @eggachecat |
Add ignore_order_func to make ignore-order operation dynamic with level
Allow custom operators to do/report some custom operations