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

feat: ToPrimitive split into ToFloatPrimitive and ToIntegerPrimitive #316

Closed
dzmitry-lahoda opened this issue Apr 9, 2024 · 7 comments
Closed

Comments

@dzmitry-lahoda
Copy link

dzmitry-lahoda commented Apr 9, 2024

Problem

Some restricted VMs/hosts/environments:

  • Zero knowledge
  • Multi Party Computation
  • Embedded
  • Deterministic Networking Game Engines, consensuses and BFT protocols
  • Banking

Does not have floating point numbers implemented and their validators/loaders (of code or compiled binary) complain or/and fail to handle code referencing f64/f32

Solution

  • Split trait into 2, so that downstream crates,
  • Or feature gate disabling floats
@dzmitry-lahoda
Copy link
Author

I would be happy to PR it solution has chances to be accepted

@dzmitry-lahoda
Copy link
Author

Other option is to gate all float methods behind float feature

@cuviper
Copy link
Member

cuviper commented Apr 10, 2024

I have no plans to start making breaking changes. For now, I think if you have such restricted needs, it might be better to just fork into a new crate for your environment.

@dzmitry-lahoda
Copy link
Author

Thanks for response.

Forking is not good option as will need fork whole crate tree built on top.

Smallest breaking change can be made by making disable of floats only if no default no-floats features is defined.
So if default features are defined on some already existing, all will be as before.

Only issue would be if all-features are used. So I doubt I saw it wide usage.

@cuviper
Copy link
Member

cuviper commented Apr 12, 2024

Features are meant to be additive. If crate A uses num-traits with some current features and uses floats, and crate B turns on the no-float feature, and then both are used as dependencies in some larger project, then A will be broken by B's choice. That's not acceptable.

Forking is not good option as will need fork whole crate tree built on top.

API changes also require the whole tree to adapt, and this crate has thousands of dependents.

You may be able to [patch] in a replacement crate, but overall I'd say that your specialized/restricted needs call for specialized crates.

@dzmitry-lahoda
Copy link
Author

Features are meant to be additive. If crate A uses num-traits with some current features and uses floats, and crate B turns on the no-float feature, and then both are used as dependencies in some larger project, then A will be broken by B's choice. That's not acceptable.

It looks very desired behavior. If somebody enables "disabling" feature, it will break all float usage in other crates, compilation will fail, which is right. Nobody to use floats.

API changes also require the whole tree to adapt, and this crate has thousands of dependents.

that is clear, anyway just tried to propose :) so need to adapt on forks or via gate(both likely).

You may be able to [patch] in a replacement crate

Patch will patch crate for all deps, so if any dep will impl float methods, they will fail to compile. So will need to patch all other crates built on top. Num based crates about fractions(ratio) and big integers(bigger than 128) are of interest.

but overall I'd say that your specialized/restricted needs call for specialized crates.

Thank you for response. I understand.

So my understanding (and usage), there are core::ops traits to be used in any context/need/vm. I just peek traits what work for me. num-traits has a lot of good traits generic enough and I use them a lot (Zero/One/ops).

But To/FromPrimitive are not generic enough, I cannot use to integer vs float primitives. Floats leaks to compilation result as part of format/display/debug routines, which breaks specialized environments.

I can use core::ops without touching float code, but I cannot do that with num-trait, which would be possible num-traits would be generic enough to be used in any context (so basically this states that breaking thousands of dependents is right thing to do to make num-trait as generic as core::ops). And after that num-rational and num-big and many other crates conforming from num-traits will work out of box for some(my) use cases.

@dzmitry-lahoda
Copy link
Author

dzmitry-lahoda commented Apr 15, 2024

Another possible breaking change tbh can be done accopanying any breaking change

https://docs.rs/num/latest/num/traits/trait.Zero.html

Zero forces to have Add which limits Zero usage. I can have Zero without ability to add.
One is also useful without ability to Mul.

Also some part of num-traits is already splits float to float mod, so To/From primitive looks like just tech dept to split.

Closing for now so. Can send PR with gate or split of traits if any will be considered as option. In general I feel that not my specialized needs are kind of things, but that num-traits are not general enough(easy to fix, so breaking change) in the end.

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