approx
approx copied to clipboard
Implement traits for `Option` and `Result`
Values of types Option and Result are frequently checked for equality in testing, but approx does not currently implement its traits on these types. This PR aims to do just that.
For Result<T, E> I have opted to require the given trait on both T and E, so that the error can also contain approximately comparable values. I'm not entirely sure about this decision, however, so I would appreciate your input. The downside of this approach is that all error types are also forced to implement the given trait, which might be asking too much of the users of this library - especially since there is no easy way (e.g. #[derive(...)]) to do it automatically.
It's worth noting that (Result<T, E> as UlpsEq)::default_max_ulps returns the maximum tolerance of T and E. This is because the return type cannot be a tuple (unlike AbsDiffEq), and it seemed reasonable to allow the greater value. If anybody has a better idea - that doesn't involve breaking changes -, please let me know.
I can separate the implementations of Option and Result into separate PRs if necessary.
@brendanzab Any chance to get this reviewed and merged?
Much more useful would be to only need Result<T: *DiffEq, E>, with errors not being equal to each other.
On second thought, this would disagree with the behaviour of PartialEq on Result, so it's not clear whether this suggestion would be too surprising. Maybe *DiffEq should not be implemented for Result.
I do think that the Option<T: *DiffEq> case is unsurprising and would be useful, which was what lead me to this PR.
Edits: Should have read the comment before checking out the commit, the problems with the bounds on E were already clearly stated, my bad.
Stumbled over the missing implementation of Option<T>, too. Unfortunately it cannot manually be implemented outside of Approx as Option is a foreign type, and AbsDiffEq is a foreign trait to my module.
In my opinion implementation of Result is less import if Optionis available as you could then write your tests like
assert_abs_diff_eq!(x.ok(), Some(...));
So how about merging that part for the time being?
Could this be added as a cargo feature, maybe marked as "experimental"? In my case I am comparing type Vec<Option<f64>> in my assertions so it's really convenient having that type supported out of the box.