Skip to content

Conversation

@chriselrod
Copy link
Collaborator

When broadcasting with array types that returned static(1):N for axes, we'd get OffsetArrays instead of Arrays. These should hopefully now generally be returning Arrays instead:

  @test identity.(static(1):5) isa Vector{Int}
  @test (static(1):5) .+ (1:3)' isa Matrix{Int}
  @test similar(Array{Int}, (static(1):(4), Base.OneTo(4))) isa Matrix{Int}
  @test similar(Array{Int}, (static(1):(4),)) isa Vector{Int}

Under this PR, if the first OptionatllyStaticUnitRange{StaticInt{1}} occurs in index 3 or later of the tuple, it'll still result in an OffsetArray, but I figured getting these cases is enough.
It'd just require copy/pasting and adding more methods if we wanted to support deeper indices with the current approach.

@chriselrod chriselrod requested a review from Tokazama March 9, 2022 20:40
@codecov
Copy link

codecov bot commented Mar 9, 2022

Codecov Report

Merging #249 (da610c6) into master (b2889ed) will decrease coverage by 0.16%.
The diff coverage is 60.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #249      +/-   ##
==========================================
- Coverage   89.31%   89.14%   -0.17%     
==========================================
  Files          11       11              
  Lines        1741     1751      +10     
==========================================
+ Hits         1555     1561       +6     
- Misses        186      190       +4     
Impacted Files Coverage Δ
src/ranges.jl 91.07% <60.00%> (-1.46%) ⬇️

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 b2889ed...da610c6. Read the comment docs.

@chriselrod chriselrod merged commit b2304bd into master Mar 10, 2022
@chriselrod chriselrod deleted the combine_axes branch March 10, 2022 01:33
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.

3 participants