num-traits icon indicating copy to clipboard operation
num-traits copied to clipboard

Float should require FloatCore

Open vadixidav opened this issue 5 years ago • 5 comments

I am currently working on converting ndarray to no_std, but this has resulted in one particular pain point: https://github.com/rust-ndarray/ndarray/issues/708#issuecomment-743914960.

The underlying issue is that ndarray uses Float currently, but Float does not require the implementation of FloatCore. This means that it is possible for a downstream crate to implement Float on a type without implementing FloatCore. Therefore, ndarray cannot "lower" the requirements to only FloatCore, since we cant guarantee that something that is Float is also FloatCore.

Due to this, ndarray may make a breaking change (thats up to @bluss though), but it might make sense to make this breaking change in num-traits instead. If this is a change that should be made at all, it should probably be done sooner rather than later to avoid more ecosystem pain. It could also be that we don't ever change it so that we don't need to worry about breaking any downstream code. From here on out crates switching to no_std will continue to be broken if they depend on Float, but if the change is made here in num-traits then it will break everyone at least once, which is also a huge pain, so perhaps we shouldn't do it at all.

Aside from this breaking issue, it would also make the most sense for something implementing Float to also implement FloatCore, otherwise upstream crates (like ndarray) supporting no_std would be unable to use just Float in their APIs that might call shared code (DRY principle) that works on std and no_std that uses FloatCore since Float doesn't implement FloatCore. So effectively these kinds of crates would have to have APIs with the bound Float + FloatCore, which is a bit awkward (but it would work!), or they might have awkward duplicate (or macro-generated) code.

Any thoughts?

vadixidav avatar Dec 12 '20 23:12 vadixidav

(This doesn't need to detract from this issue, but:) Making ndarry no_std is a breaking change in ndarray anyway, because we're going to make the "no default features" version of ndarray be no-std. (This is a quite usual way to do it and it looks like the most reasonable solution to me.)

bluss avatar Dec 12 '20 23:12 bluss

Also a little curious. I think the reason why Float does not implement FloatCore is to express that FloatCore is the no_std version of Float, so there will be differences in implementation or functionality (even if not now)? If we want to express that FloatCore is the core function of Float, then Float should implement FloatCore, personal opinion.

SparrowLii avatar Dec 13 '20 11:12 SparrowLii

It is hard to make breaking changes in this crate. Possible breaking changes are tracked here: #47

bluss avatar Dec 13 '20 18:12 bluss

Due to this, ndarray may make a breaking change (thats up to @bluss though), but it might make sense to make this breaking change in num-traits instead.

There's no "instead", really -- if we make a breaking change in num-traits with a requisite semver bump (1.0 or otherwise), ndarray will also have to break to follow along.

cuviper avatar Dec 14 '20 22:12 cuviper

@cuviper True, but if a breaking change is made here at some point, it would be better to include it to avoid this situation across multiple downstream crates trying to use no_std. Otherwise it probably isn't worth it.

vadixidav avatar Dec 14 '20 23:12 vadixidav