Skip to content

Remove num_traits dependency. - #107

Merged
jonaspleyer merged 2 commits into
brendanzab:masterfrom
ElliottjPierce:remove-num_traits-dep
Jul 9, 2026
Merged

Remove num_traits dependency.#107
jonaspleyer merged 2 commits into
brendanzab:masterfrom
ElliottjPierce:remove-num_traits-dep

Conversation

@ElliottjPierce

@ElliottjPierce ElliottjPierce commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

It would be nice if we didn't need to pull in num_traits to use this.

The only reason I can think of to not do this if it it would somehow not work on older rust versions.
But, I don't think this would actually cause any problems, and I didn't see a MSRV anywhere.

(cargo test passes locally. Let me know if there's any other test's you'd like me to run or anything!)

@ElliottjPierce
ElliottjPierce marked this pull request as draft July 2, 2026 02:27
@ElliottjPierce
ElliottjPierce marked this pull request as ready for review July 2, 2026 02:30
@jonaspleyer

Copy link
Copy Markdown
Collaborator

Hi and thanks for the addition. I was not aware that the num_traits dep is actually not actively utilized anymore. I have reviewed the commits and to me this all seems fine. Do you have an explanation as to why the traits were there? Possible compatibility for older Rust versions? I am willing to break that, depending on the particular version.

@ElliottjPierce

Copy link
Copy Markdown
Contributor Author

@jonaspleyer Thanks!

To be totally honest, I have no idea why this dependency was there in the first place. I thought maybe it was for compatibility with older rust versions, but all the methods I checked were stable since 1.0, but I could have missed something. The only thing I can think of is that maybe when this was first written, it was based on blanket impls instead of macros. And when they moved to macros, they just brought this with them. But I really don't know. I'd be happy to spend some time looking into the history of it if it's important to you?

@jonaspleyer

Copy link
Copy Markdown
Collaborator

The history is not too important to me to be honest. We should simply make sure that we do not accidentally break any form of compatibility. But I think that I will merge the code as is. Because the traits only appear inline in a function where it is encapsulated. Also, removing trait bounds on generics is generally backwards-compatible.

@jonaspleyer
jonaspleyer merged commit 16ca8d6 into brendanzab:master Jul 9, 2026
19 checks passed
@brendanzab

Copy link
Copy Markdown
Owner

If it’s of any interest, it seems like num_traits was added in #33, in order to fix #32 (used GitHub’s blame feature to track this down, as I was curious).

@ElliottjPierce

Copy link
Copy Markdown
Contributor Author

Oh interesting! That's strange, too though. I wonder why they needed core::num::Float back then...

Anyway, should still be fine. I just tested it and it compiled for thumbv6m-none-eabi, so it should be good. Might be worth adding that target to CI if you want more tests for no_std platforms.

@brendanzab

Copy link
Copy Markdown
Owner

It appears like the original usage of core::num::Float was added in b6221de, but I'm not sure it was actually used at that time!

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

Successfully merging this pull request may close these issues.

3 participants