deepdiff icon indicating copy to clipboard operation
deepdiff copied to clipboard

Allow using math.isclose for Python 3.5+

Open stevenjackson121 opened this issue 7 years ago • 5 comments

Python 3.5 introduces math.isclose and I've found it very useful in several contexts.

I would be happy to contribute a PR for this enhancement if you're open to including this functionality and give guidance on your preferred API.

I would love to be able to use this instead of significant_digits for comparing numbers which may are very large in magnitude and differences could be neglibible even if they're different in absolute terms by more than 1 (1,000,000,005 is close to 1,000,000,004), or for very small numbers as described in this issue

Are you open to including this as an option, and if so what would the preferred api be?

I recommend:

  1. Adding use_isclose, rel_tol and abs_tol optional arguments.
  2. If either rel_rol or abs_tol is provided, use_isclose is assumed to be True, otherwise it defaults False (preserving current behavior).
  3. Setting significant_digits at the same time as use_isclose is True (either explicitly or implicitly) would be an error.

stevenjackson121 avatar Aug 17 '18 21:08 stevenjackson121

@stevenjackson121 This is a very cool idea. I have been thinking about how to use the math.isclose. The main problem is how to do that during the "hashing" phase where numbers are trimmed to their significant digits before getting hashed and at that point we don't know which number value in the future during the diffing part we will be comparing to. The hashing is only used when "ignore_order=True". So if we want to use math.isclose, we will hit some unexpected results when ignore_order=True.

seperman avatar Mar 20 '19 22:03 seperman

Hi @stevenjackson121 Have you checked the combination of significant_digits and scientific format notation? https://zepworks.com/deepdiff/5.0.0/numbers.html#number-format-notation

seperman avatar Jun 23 '20 19:06 seperman

It looks like it is now using is_close with abs_tol whenever math_epsilon is not none https://github.com/seperman/deepdiff/blob/7e5e9954d635423d691f37194f3d1d93d363a128/deepdiff/diff.py#L1116

However It would be great to add another parameter that would allow to use is_close with rel_tol instead of abs_tol As a workaround I am currently inheriting from DeepDiff and adding a parameter in the derived class:

class DeepDiff(deepdiff.DeepDiff):
    """Class inherited from deepdiff.DeepDiff that adds the parameter rel_tol"""

    def __init__(self, t1,  t2, rel_tol=None, **kwargs):
        # adding rel_tol attribute before calling the baseclasse init as the diffeference gets 
        # computed within the constructor
        self.rel_tol = rel_tol
        super().__init__(t1,  t2, **kwargs)

    def _diff_numbers(self, level):
        if self.rel_tol is not None:
            if not is_close(level.t1, level.t2, rel_tol=self.rel_tol):
                self._report_result("values_changed", level)
        else:
            super()._diff_numbers(level)

It would be good to get the change integrated in the original class by adding this argument to the constructor.

With this change DeepDiff(10050001,10049999, rel_tol=1e-6) gives no difference while DeepDiff(10050001,10049999, math_epsilon=0.1) and DeepDiff(1005001,1004999, significant_digits=2, number_format_notation="e") both detect a difference

martinResearch avatar Jan 20 '22 18:01 martinResearch

@seperman I can prepare a pull request to add an argument rel_tol to the DeepDiff constructor if that can help.

martinResearch avatar Jan 21 '22 15:01 martinResearch

Hi Martin, That would be great! Thank you,

Sep Dehpour

On Jan 21, 2022, at 8:04 AM, Martin de La Gorce @.***> wrote:

 @seperman I can prepare a pull request to add an argument rel_tol to the DeepDiff constructor if that can help.

— Reply to this email directly, view it on GitHub, or unsubscribe. Triage notifications on the go with GitHub Mobile for iOS or Android. You are receiving this because you were mentioned.

seperman avatar Jan 21 '22 15:01 seperman