NaNMath.jl icon indicating copy to clipboard operation
NaNMath.jl copied to clipboard

Implement `findmin`, `findmax`, `argmin`, `argmax`

Open jd-foster opened this issue 2 years ago • 3 comments

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.

jd-foster avatar Sep 01 '22 14:09 jd-foster

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

sethaxen avatar Sep 01 '22 14:09 sethaxen

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
@@            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 Coverage Δ
src/NaNMath.jl 98.20% <100.00%> (+1.17%) :arrow_up:

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

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

codecov[bot] avatar Sep 01 '22 14:09 codecov[bot]

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.

jd-foster avatar Sep 01 '22 14:09 jd-foster