Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add similar for mixed axis types #470

Merged
merged 3 commits into from
Mar 4, 2023
Merged

Add similar for mixed axis types #470

merged 3 commits into from
Mar 4, 2023

Conversation

sethaxen
Copy link
Collaborator

@sethaxen sethaxen commented Mar 4, 2023

This is a (particularly ugly) fix for #464 based on #464 (comment) and #464 (comment). It allows DimUnitRange to be mixed with Integer and/or Base.OneTo axes, so long as a DimUnitRange appears within the first 4 axes, which should cover the most common cases. Fixes #464

@mcabbot I ended up dispatching also on Integer types for axes because otherwise we need to also implement overloads to hit similar(::AbstractArray, ::Tuple) for all of these combinations, which seemed unnecessary.

Still need to exhaustively test, but now this works:

julia> da = DimArray(randn(5, 2, 10), (X(1:5), Y(1:2), Z(1:10)));

julia> similar(parent(da), axes(da)) isa DimArray
true

julia> similar(parent(da), (Base.OneTo(10), axes(da)...)) isa Array
true

julia> similar(parent(da), (axes(da)..., Base.OneTo(10))) isa Array
true

julia> slices = eachslice(da; dims=X);

julia> da_slices = rebuild(da; data=slices, dims=dims(axes(slices)));

julia> stack(da_slices) isa DimArray
true

@codecov-commenter
Copy link

codecov-commenter commented Mar 4, 2023

Codecov Report

Merging #470 (ddcbaf5) into main (be957d9) will increase coverage by 0.06%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #470      +/-   ##
==========================================
+ Coverage   89.64%   89.70%   +0.06%     
==========================================
  Files          38       38              
  Lines        2636     2653      +17     
==========================================
+ Hits         2363     2380      +17     
  Misses        273      273              
Impacted Files Coverage Δ
src/array/array.jl 96.45% <100.00%> (+0.48%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@rafaqz
Copy link
Owner

rafaqz commented Mar 4, 2023

Thanks. Its unfortunate we have to do these "at least the first four args work" hacks, but better than nothing working.

@sethaxen sethaxen marked this pull request as ready for review March 4, 2023 19:44
@sethaxen sethaxen requested a review from rafaqz March 4, 2023 20:02
Copy link
Owner

@rafaqz rafaqz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, looks good

@rafaqz rafaqz merged commit d28fbcb into main Mar 4, 2023
@sethaxen sethaxen deleted the similarmixed branch March 4, 2023 22:06
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.

Base.stack fails if DimArray and non-DimArray mixed
3 participants