JuliaLang / Compat.jl

Compatibility across Julia versions
Other
142 stars 117 forks source link

`sort` for NTuple and other iterables #804

Closed mtfishman closed 11 months ago

mtfishman commented 11 months ago

Closes #803.

codecov[bot] commented 11 months ago

Codecov Report

Merging #804 (dc7d33a) into master (cc4763c) will decrease coverage by 0.12%. The diff coverage is 92.30%.

:exclamation: Current head dc7d33a differs from pull request most recent head 38ed62b. Consider uploading reports for the commit 38ed62b to get more accurate results

@@            Coverage Diff             @@
##           master     #804      +/-   ##
==========================================
- Coverage   92.54%   92.42%   -0.12%     
==========================================
  Files           2        2              
  Lines         295      317      +22     
==========================================
+ Hits          273      293      +20     
- Misses         22       24       +2     
Files Changed Coverage Δ
src/Compat.jl 92.90% <92.30%> (-0.17%) :arrow_down:
mtfishman commented 11 months ago

I improved the use of import, and switched over to just using using and overloading with Base.sort.

I also switched over to that style for code I introduced in #799 for overloading round/trunc/ceil/floor, hope that's ok to do in this PR.

Finally, I realized I dropped the definition:

sort(v::AbstractVector; kws...) = sort!(copymutable(v); kws...)

which was included in https://github.com/JuliaLang/julia/pull/46104. It doesn't seem to be totally necessary since it is essentially the same as the more general sort(v), but skips some checks that aren't needed for AbstractVector subtypes. Seems best to just directly have the same code as Base. EDIT: Removed in the latest version since it is already in Base for older versions of Julia.

ericphanson commented 8 months ago

This should probably be reverted as it was reverted upstream: https://github.com/JuliaLang/julia/pull/52010

martinholters commented 7 months ago

We probably need to deprecate this here instead of reverting completely.

martinholters commented 7 months ago

811