dasp icon indicating copy to clipboard operation
dasp copied to clipboard

Rename `FloatSample` and `SignedSample` to `Float` and `Signed`

Open mitchmindtree opened this issue 8 years ago • 4 comments

They already live in lib.rs, and can be accessed as sample::Float and sample::Signed.

mitchmindtree avatar Mar 28 '16 03:03 mitchmindtree

I'd like to petition against this modification. I use num::Float pretty heavily for my frequency domain processing applications, and the namespace collision that happens when I'm trying to keep certain traits in scope at certain times is pretty confusing. Basically, there are many many applications for a Float that do not involve samples at all, but rather some other kind of data.

(edit) For example:

let one: S::Float = 1.0.to_sample();
((one - phase.to_float_sample()) * (two_thirds + (one_third * v.cos())) 
    + (one / pi_2) * v.sin()).to_sample::<S>()

causes an error because I have the Float in scope. I have to have the Float trait in scope because of a later piece of the file that requires some num::Float operations—so this namespace collision necessitates either lots of type annotation in function calls, or separate files.

andrewcsmith avatar Jun 09 '16 17:06 andrewcsmith

I'd forgotten about this issue!

What are your thoughts on the addition of Float in #38? I originally added it because I thought it would be a necessary bound for the S type passed to window::Type::at_phase, however it seems that Sample is enough.

I'll remove the Float trait from that PR as we can always add it later on if necessary and in retrospect, it should really be done in a separate PR anyway, especially as it would be a breaking change.

Also, I find that I rarely have name-spacing issues like this in rust as rust provides so many easy ways around them. For example, conflicting trait names can be imported as another name and UFCS allows calling methods as functions (so if there is a conflict between num::Float::sin and sample::Float::sin for f32, rather than calling f.sin(), we can distinguish which trait we want via num::Float::sin(f) or sample::Float::sin(f).

mitchmindtree avatar Jun 09 '16 23:06 mitchmindtree

@andrewcsmith I've removed the Float trait from #38.

mitchmindtree avatar Jun 09 '16 23:06 mitchmindtree

Great, thanks. Yes, of course, Rust has plenty of ways around namespacing issues, and I've gotten my code working just fine. It's mostly just cause the num crate is so common.

andrewcsmith avatar Jun 10 '16 05:06 andrewcsmith