Replace `round` with `div`

[FedericoStra]

Minor change: isn't it probably better to use integer division div instead of rounding the floating point result?

It should be equivalent in common cases, more correct in extreme cases, and faster.

Regarding correctness in extreme cases, one has for instance

julia> len = 4Int(maxintfloat()) + 3
36028797018963971

julia> round(Int, (len-1)/2, RoundDown)
18014398509481984

julia> div(len-1, 2, RoundDown)
18014398509481985

[codecov[bot]]

Codecov Report

Merging #298 (a2f591c) into master (83ab52e) will increase coverage by 0.01%. The diff coverage is 100.00%.

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

@@            Coverage Diff             @@
##           master     #298      +/-   ##
==========================================
+ Coverage   96.36%   96.38%   +0.01%     
==========================================
  Files           5        5              
  Lines         440      442       +2     
==========================================
+ Hits          424      426       +2     
  Misses         16       16              

Impacted Files	Coverage Δ
src/OffsetArrays.jl	98.28% <100.00%> (+0.01%)	:arrow_up:

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 83ab52e...aa6b88b. Read the comment docs.

[jishnub]

Perhaps this should only be added for julia v1.6+, maybe something like

if VERSION < v"1.6"
   _halfroundInt(v, r) = round(Int, v/2, r)
else
   _halfroundInt(v, r) = div(v, 2, r)
end

[FedericoStra]

Actually, reading here I believe div(x, y, r::RoundingMode) was already present in 1.4.2. Maybe I should change the check to VERSION < v"1.4.2"?

Julia 1.3.1 appeared to have RoundingMode, but I think it was used only in round.

[jishnub]

If you can verify that it works on 1.4.2, then sure

[FedericoStra]

If you can verify that it works on 1.4.2, then sure

Unfortunately I have no way to verify it locally on my PC. I can try to modify the CI tests on the forked repo.

[jishnub]

Anyway, I'm not too sure if we need to backport bugfixes to outdated minor versions, as people are strongly encouraged to upgrade to 1.6. Let's see what @johnnychen94 thinks.

[FedericoStra]

In the meantime I ran the tests with the matrix julia-version: ['1.0', '1.3', '1.4', '1.6', '1', 'nightly']: https://github.com/FedericoStra/OffsetArrays.jl/actions/runs/2446864183.

Both versions 1.3 and 1.4 failed because of some other BoundsError in doctests...

[jishnub]

Perhaps the doctests need only be run on v1.6+, I'll create a PR for that. Ultimately, we're not generating the docs using older versions. The display formats have changed subtly between versions, leading to these errors.

[jishnub]

Could you rebase on https://github.com/JuliaArrays/OffsetArrays.jl/pull/302, or incorporate the changes in your PR and see if the doctests pass with these? Locally, this makes doctests pass on v1.3 and v1.4 for me.

[FedericoStra]

I merged #302. The doctests still fail due to a whitespace in the error message:

┌ Error: doctest failure in ~/work/OffsetArrays.jl/OffsetArrays.jl/src/axes.jl
│ 
│ ```jldoctest ior
│ julia> using OffsetArrays: IdOffsetRange
│ 
│ julia> ro = IdOffsetRange(1:3, -2)
│ IdOffsetRange(values=-1:1, indices=-1:1)
│ 
│ julia> axes(ro, 1)
│ IdOffsetRange(values=-1:1, indices=-1:1)
│ 
│ julia> ro[-1]
│ -1
│ 
│ julia> ro[3]
│ ERROR: BoundsError: attempt to access 3-element IdOffsetRange{Int64, UnitRange{Int64}} with indices -1:1 at index [3]
│ ```
│ 
│ Subexpression:
│ 
│ ro[3]
│ 
│ Evaluated output:
│ 
│ ERROR: BoundsError: attempt to access 3-element IdOffsetRange{Int64,UnitRange{Int64}} with indices -1:1 at index [3]
│ Stacktrace:
│  [1] throw_boundserror(::IdOffsetRange{Int64,UnitRange{Int64}}, ::Tuple{Int64}) at ./abstractarray.jl:538
│  [2] checkbounds at ./abstractarray.jl:503 [inlined]
│  [3] getindex(::IdOffsetRange{Int64,UnitRange{Int64}}, ::Int64) at /home/runner/work/OffsetArrays.jl/OffsetArrays.jl/src/axes.jl:204
│  [4] top-level scope at /home/runner/work/OffsetArrays.jl/OffsetArrays.jl/test/runtests.jl:42
│ 
│ Expected output:
│ 
│ ERROR: BoundsError: attempt to access 3-element IdOffsetRange{Int64, UnitRange{Int64}} with indices -1:1 at index [3]
│ 
│   diff =
│    Warning: Diff output requires color.
│    ERROR: BoundsError: attempt to access 3-element IdOffsetRange{Int64, UnitRange{Int64}} IdOffsetRange{Int64,UnitRange{Int64}} with indices -1:1 at index [3][3]
│    Stacktrace:
│     [1] throw_boundserror(::IdOffsetRange{Int64,UnitRange{Int64}}, ::Tuple{Int64}) at ./abstractarray.jl:538
│     [2] checkbounds at ./abstractarray.jl:503 [inlined]
│     [3] getindex(::IdOffsetRange{Int64,UnitRange{Int64}}, ::Int64) at /home/runner/work/OffsetArrays.jl/OffsetArrays.jl/src/axes.jl:204
│     [4] top-level scope at /home/runner/work/OffsetArrays.jl/OffsetArrays.jl/test/runtests.jl:42
└ @ Documenter.DocTests ~/work/OffsetArrays.jl/OffsetArrays.jl/src/axes.jl

[jishnub]

Could you point me towards the branch on which tests are failing? The latest fix on your fork's master doesn't seem to incorporate #302

[FedericoStra]

Could you point me towards the branch on which tests are failing? The latest fix on your fork's master doesn't seem to incorporate #302

Sorry my bad, I wrongly assumed #302 was already merged in this repo's master.

The tests are now succeeding.

[jishnub]

Great! In that case, could you update the compat bound?

[jishnub]

Could you also bump the patch version? I'll merge and tag