atol and rtol in isapprox

[jishnub]

This adds atol and rtol parameters to the isapprox defined here. This method is committing type-piracy here (and perhaps harms pre-compilation, as it is overwriting the other method), so perhaps this should be removed in a future breaking version. The breakage comes from the fact that the isapprox defined in IntervalSets throws an error when comparing closed and open intervals, whereas this version doesn't.

[codecov]

Codecov Report

Base: 86.09% // Head: 86.10% // Increases project coverage by +0.00% 🎉

Coverage data is based on head (90f5ebd) compared to base (8bce5e6).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files

@@           Coverage Diff           @@
##           master     #122   +/-   ##
=======================================
  Coverage   86.09%   86.10%           
=======================================
  Files          31       31           
  Lines        2496     2497    +1     
=======================================
+ Hits         2149     2150    +1     
  Misses        347      347           

Impacted Files	Coverage Δ
src/domains/interval.jl	94.68% <100.00%> (+0.02%)	⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

[daanhb]

Throwing an error when comparing open and closed domains seems like a sensible thing to do. Type-piracy aside, I think we should update DomainSets accordingly.

[jishnub]

Removing this method from DomainSets would make such a comparison throw an error, as the method in IntervalSets does that already. However, this is technically breaking, so ideally this should be removed in the next minor release

[daanhb]

Sure, we can make version 0.6. I think @dlfivefifty is looking at this issue in #123 so I won't touch anything.

[jishnub]

Perfect. I would suggest making a v0.5 release with this PR, so that isapprox(a, b; rtol), which works currently after loading IntervalSets, is not broken after subsequently loading DomainSets although the method is overwritten. Following this, the method may be removed from DomainSets altogether in v0.6

Edit: Sorry, it seems the method is not overwritten after all. Only the isapprox(a, b) one is overwritten, not the one with kwargs. In that case, this PR might not be necessary.

[dlfivefifty]

Note this is now in IntervalSets.jl: JuliaMath/IntervalSets.jl#125

I think throwing an error makes no sense; see my PR JuliaMath/IntervalSets.jl#129

[daanhb]

Meanwhile, that definition seems to have disappeared. I have also removed the tests for approximate equality of intervals, since that is defined and tested in IntervalSets. We still have to implement/improve isapprox more generally for domains.