Skip to content

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