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

Conversion/Casts considerations #88

Open
Patiga opened this issue Nov 29, 2022 · 3 comments
Open

Conversion/Casts considerations #88

Patiga opened this issue Nov 29, 2022 · 3 comments

Comments

@Patiga
Copy link
Contributor

Patiga commented Nov 29, 2022

Context: I'm using vek in combination with fixed point numbers. In case you are curious, the project I'm working on is twmap (and the few other things I build on top of it).

Vec2 currently has numcast as its main method of conversion afaik.
numcast relies on Numcast, which fixed doesn't implement (reasoning).

Conversion methods which I think could be added

  • Two methods using ToPrimitive and FromPrimitive (I guess also AsPrimitive if you want to be thorough)
  • From, Into, TryFrom and TryInto based conversion. I personally don't need them yet, but I can see me needing them in the future. Implementing the trait is not possible, but methods using the traits could help.
  • The az crate has a nice set of conversion traits. These are not implemented generically and thus can be implemented. fixed supports them, but otherwise az doesn't have that many downloads.

Like last time, I'm willing to put my own time into this, but I won't stop you if you judge that it's worth your time :)
I'll continue opening issues for non-trivial things, as you suggest in the README

@yoanlcq
Copy link
Owner

yoanlcq commented Nov 30, 2022

I think this time a PR would be appropriate (that would also allow you to test before changes are published).

Some guidelines:

  • I'd like not to depend explicitly on fixed; the solution should work for any "number type" crate (as long as that crate implements traits from ubiquitous crates such as num_traits, but for that purpose I'd say az may count as well)
  • If we do have to depend on fixed (which I hope we don't), then this should be gated under a non-default feature.

The point is, I'd like to avoid a situation where vek "blesses" a few specific crates that are not widely used, adding to its dependencies; there's a threshold where it might be more elegant to have a separate crate (such as "vek-fixed-interop", I'm exagerrating but that's the idea)

Methods can be added as long as they are general enough (so stuff based on FromPrimitive/ToPrimitive).
The az crate looks ok as a "common ground" that number types can implement, but I'm hesitating because it looks, although well-designed, quite specific and niche.

In your specific project, you might find it more convenient to have "extension traits" implemented for all vector types, where you add conversion methods as you see fit (then put the trait impl into a macro re-used for all vector types).
The map() method can make this easy; there should be no need to get as "sophisticated" as the current macros in vek's internals.

If I understood correctly it's just a matter of using To/FromPrimitive to cast to f32/f64 and that's it? Then helper methods could be added to vectors as you suggest, but I'd like to avoid getting further than that.

@Patiga
Copy link
Contributor Author

Patiga commented Dec 2, 2022

Would you consider adding az as an optional dependency, or would you rather avoid that as well?
I do understand that optional dependencies can also kind of bloat, so I wouldn't mind if you aren't a fan of that

@yoanlcq
Copy link
Owner

yoanlcq commented Dec 2, 2022

I think I'm OK with az as an optional dependency.

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