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: [wip] ordered-float feature options #492

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

gechelberger
Copy link

Related to issue #208

Working on optional lib features to handle ordered_float wrapper types natively.

This draft is probably 90% of the way there but there are a few design decisions still to make, and a lot of polishing to be done.

@gechelberger gechelberger marked this pull request as draft November 4, 2024 06:03
@iliekturtles
Copy link
Owner

Thanks for starting this PR. I allowed test actions to run and will apologize in advance for delays in reviewing!

@gechelberger
Copy link
Author

No worries. I think I fixed the feature gates that tripped up CI.

OrderedFloat seems to be fully implemented - pending more detailed testing.
NotNan can not be implemented the same way since it is not num_traits::float::Float.

I'm not sure if it's better to just yank NotNan entirely in favor of OrderedFloat or to break the macros up into smaller groups of operations that each is compatible with (including restructuring complex a bit).

Related to #493, normally I would want to resolve that on another PR first and then merge this on top, but I think I'm going to have to restructure Complex and Float a bit for them to co-exist with OrderedFloat and NotNan anyways, so I'm not sure.

@gechelberger
Copy link
Author

gechelberger commented Nov 5, 2024

A couple points of interest:

QuantityArguments traits: I haven't fully wrapped my head around how they fit into the framework yet and what the correct types should be.

NotNan tests NotNan needs its own whole test suite since it panics on a bunch of "reasonable" arbitrary inputs like any negative for sqrt.

ConstZero for NotNan ConstZero can't be implemented for NotNan without an unsafe block (which in this context is completly safe)

There are definitely other things to sort out, but these are the ones that could most benefit from some input.

Thanks!

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.

2 participants