Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Simplify the trait bounds in the geom crate. #281

Closed
nical opened this issue Dec 30, 2017 · 2 comments
Closed

Simplify the trait bounds in the geom crate. #281

nical opened this issue Dec 30, 2017 · 2 comments

Comments

@nical
Copy link
Owner

nical commented Dec 30, 2017

In #279 that landed just after the epic generic-ization of the geom crate, I ended up adding some Float + FloatConst + ApproxEq bounds that bubbled up in more places than ideal. It would be nice if we could figure out some trickery to have only one trait just to keep things easier to maintain/read/document.

Ideally Float could depend on FloatConst in num_traits, I filed rust-num/num-traits#20, we'll see if that's possible.

Also, I think that we can get away with implementing something like approx_eq with only Float without requiring a trait.

Or we could just add our own Float trait that inherits Float + FloatConst + ApproxEq (and whatever else we need to add in the future), but that would limit the amount of types that can be used to f32/f64 and whatever types are defined by the user of the crate, but no type exposed by a third party crate due to orphan rules (although ApproxEq currently already causes has this problem).

@pizzaiter do you have use cases outside of f32 and f64 that we should watch out for?

@nical
Copy link
Owner Author

nical commented Jan 7, 2018

Fixed in #290.

@nical nical closed this as completed Jan 7, 2018
@pizzaiter
Copy link
Contributor

For the record, my motivation was that I wanted to use f64. The choice of Float as base trait already pretty much means {f32, f64} in practice even without any orphan rule problems. I think it would be great to some day support BigRational, but I guess this is out of scope for the foreseeable future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants