Comments (6)
I've created an issue for making the change to FloatDiff. I'm going to leave FloatEq in terms of references, since I think that's more idiomatic.
from float_eq-rs.
Thank you for your interest!
I like the idea of being able to compare Iterator
s, but I would like to keep the parameter as &self
. Conceptually, FloatEq
mirrors the standard PartialEq
, which means that it compares two borrowed values rather than consuming either of them. I would also really like to avoid requiring the input to be Copy
-able, since it would be good to support comparing large values without requiring a copy or clone.
Is it possible to implement the existing trait for Iterator
rather than IntoIterator
to achieve what you're after? I can see the potential for an implementation over two different but compatible iterators as well, which sounds like it could be handy.
from float_eq-rs.
I've thought about it a little more, and I understand where you're coming from, here are my thoughts:
- I think taking a shared reference prevents implementing over
Iterator
s no matter what.IntoIterator
requires a value andIterator
requires a mutable reference to advance. - Taking
self
by value doesn't require the type to beCopy
. For example, you couldimpl<'a> FloatDiff for &'a Foo
. This is how some other traits, likestd::ops::Add
work. - I'm not sure this is directly comparable to
PartialEq
.eq
always returnsbool
, butFloatDiff
, for example, may need to return something lifetime-bound, which I think is not possible without taking by value (or adding an explicit lifetime to the trait, but I don't think that's the right solution).
That said, I don't think returning an iterator is possible unless it is Copy
or Clone
, since Debug::fmt
takes a shared reference. Each call to Debug::fmt
would clone the iterator and print the contents.
I do want to point out that without changing the traits it would be possible to implement over slices, however due to the limitation I mentioned about lifetimes, the output would need to be allocated and wouldn't work in no_std
.
from float_eq-rs.
Oh! I must admit, I haven't thought that deeply about why PartialEq
is defined by &self
rather than as self
. You've persuaded me that it might be worth changing as per your original suggestions :)
I think that my main potential worry is to do with the API becoming harder to use if different levels of borrowing start to conflict with one another at compile time (as this thread seems to suggest), but it's easy enough to write tests to cover those cases to check. The other thing is that I don't understand how these kinds of trait definitions might interact when they overlap yet, so that's also as much a learning exercise on my part too.
If you wished to investigate this yourself, I'd be happy to look at a PR, with the caveat that I'm new to using git/github so it might take me longer than you're used to! Otherwise, I can have a go at implementing it when I get the chance :)
from float_eq-rs.
Right, I've done some more thinking/research and I have some further thoughts, my last reply may have been a bit hasty. I've not thought about the implications for Iterator
implementations yet since I'm trying to get my head around the basics first. Apologies in advance, I'm also quite new to Rust in general.
- I'd like the traits to be idiomatic Rust unless there's a very good case for them not to be.
- Since I don't have much direct experience using traits like these yet, I'd like them to be patterned after how the standard library traits work, on the assumption that those are de facto idiomatic.
- Taking
self
by value doesn't require the type to beCopy
. For example, you couldimpl<'a> FloatDiff for &'a Foo
. This is how some other traits, likestd::ops::Add
work.
Makes sense, thank you for explaining. If I understand the implications correctly, binary operations using a non-borrowed self
allow for the value to potentially be mutable, which would enable implementations over types like IntoIterator
and optimisations where their value is mutated in place to then be used as the output, whereas using &self
constrains you to always leave the original object intact and needing to allocate a new object to return (hence your next point and the one about slices). It seems that Serde agrees as well, in a somewhat different context.
- I'm not sure this is directly comparable to
PartialEq
.eq
always returnsbool
, butFloatDiff
, for example, may need to return something lifetime-bound, which I think is not possible without taking by value (or adding an explicit lifetime to the trait, but I don't think that's the right solution).
This also makes sense - I agree that FloatDiff
is more of a binary op like Add
rather than being like PartialEq
, and it is intended to be used by itself as well as part of debugging output. Thus I think it might be worth looking into changing its method parameters to be by value (both self
and other
).
In terms of FloatEq
, I think I'd still prefer to keep that as &self
, following PartialEq
. I think that taking self
by value also necessitates the ne_*
methods requiring that it be Sized
, and I'm unsure of the implications of that as well.
The FloatEqDebug
trait feels more like a binary op, but it also requires that FloatEq
be implemented for its types. I think I'd rather leave that one as by &self
since that's what FloatEq
requires, and now that I think about it, the way that the asserts work using it probably precludes consuming IntoIterator
and similar values as well, as the debug output requires the original values being compared to still exist.
Hopefully that makes sense?
from float_eq-rs.
I completely agree that FloatEq
can (and should) take &self
.
You may be right about the implications in asserts (though shared references are Copy
so it may work out). That might be a detail that works itself out after implementing something. Another option, if none of this seems to work particularly well, is a function (or iterator extension trait) and new set of asserts specifically for iterators. I realized that in the standard library, Iterators do not implement PartialEq
, and instead have their own eq
function which takes arguments by value.
from float_eq-rs.
Related Issues (20)
- Trait impl fallbacks for non-floats HOT 1
- Debug bounds checks HOT 1
- Blanket trait impls for Result HOT 1
- FloatEq impls over slices HOT 1
- Add an option to control what is #[derive]'d on derived ulps types HOT 1
- Ulps types are poorly specified for composite types HOT 2
- Generalise trait impls across array types of any size HOT 1
- Additional relative checks HOT 1
- Design: implementing traits over enums (e.g. Result)
- Deriving extension traits over generic types
- Allow for custom #[derive] traits when deriving Ulps and DebugUlpsDiff types HOT 1
- Print float values in asserts in non-scientific format
- Use with Complex<T> should be clarified HOT 10
- Derived ulps_tol not usable in other modules because of private fields HOT 3
- Consider how ULPs should deal with special values HOT 5
- Derive support for custom types HOT 1
- Comparison of heterogeneous types HOT 2
- Add unit tests to FloatEqDebug implementations HOT 2
- Implement traits over standard Rust types HOT 1
- Derive FloatUlps HOT 1
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from float_eq-rs.