JuliaMath / NaNMath.jl

Julia math built-ins which return NaN and accumulator functions which ignore NaN
Other
52 stars 26 forks source link

Implement `findmin`, `findmax`, `argmin`, `argmax` #62

Open jd-foster opened 1 year ago

jd-foster commented 1 year ago

This is an attempt at building from #53 (and addressing issue #31) that aims to follow the existing style of code and docstrings of NaNMath. I'm going for clarity in both rather than generality or performance; in fact, deliberately restricting dispatch to AbstractFloat types. A lot of credit due on this to @sethaxen since I've adapted his PR tests and docstrings.

sethaxen commented 1 year ago

How does this PR differ from #53? If the difference is small, why not add to #53?

codecov[bot] commented 1 year ago

Codecov Report

Patch coverage: 100.00% and project coverage change: +1.17 :tada:

Comparison is base (65a5928) 97.02% compared to head (b3f3f12) 98.20%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #62 +/- ## ========================================== + Coverage 97.02% 98.20% +1.17% ========================================== Files 1 1 Lines 101 167 +66 ========================================== + Hits 98 164 +66 Misses 3 3 ``` | [Impacted Files](https://codecov.io/gh/JuliaMath/NaNMath.jl/pull/62?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaMath) | Coverage Δ | | |---|---|---| | [src/NaNMath.jl](https://codecov.io/gh/JuliaMath/NaNMath.jl/pull/62?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaMath#diff-c3JjL05hTk1hdGguamw=) | `98.20% <100.00%> (+1.17%)` | :arrow_up: | Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaMath). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaMath)

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

jd-foster commented 1 year ago

The main difference is it doesn't use the mapfoldl machinery; this is a style choice in some ways, since it wasn't clear to me how that worked, and might also be a challenge to other contributors.

Happy to make this a pull request to #53 instead, but want to run the check on this separately first.