Breaking: Include refdims as columns in DimTable

[sethaxen]

This PR adds a keyword refdims to DimTable, defaulting to the refdims of the input. All refdims are included as table columns. Fixes #884

Potentially breaking changes:

• dims field added to DimTable
• refdims columns included by default

Example

julia> using DimensionalData, Tables

julia> using DimensionalData, Tables

julia> ds = DimStack((; x=DimArray(ones(5, 3), (X, Y)), y=DimArray(zeros(5), X)))  # current behavior unchanged
┌ 5×3 DimStack ┐
├──────────────┴───────────────────── dims ┐
  ↓ X, → Y
├────────────────────────────────── layers ┤
  :x eltype: Float64 dims: X, Y size: 5×3
  :y eltype: Float64 dims: X size: 5
└──────────────────────────────────────────┘

julia> Tables.columntable(DimTable(ds))  # without refdims, current behavior unchanged
(X = [1, 2, 3, 4, 5, 1, 2, 3, 4, 5, 1, 2, 3, 4, 5], Y = [1, 1, 1, 1, 1, 2, 2, 2, 2, 2, 3, 3, 3, 3, 3], x = [1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0], y = [0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0])

julia> Tables.columntable(DimTable(ds[Y(3)]))  # refdims included by default
(X = [1, 2, 3, 4, 5], Y = [3, 3, 3, 3, 3], x = [1.0, 1.0, 1.0, 1.0, 1.0], y = [0.0, 0.0, 0.0, 0.0, 0.0])

julia> Tables.columntable(DimTable(ds[Y(3)]; refdims=()))  # manually opt out of refdims being included
(X = Base.OneTo(5), x = [1.0, 1.0, 1.0, 1.0, 1.0], y = [0.0, 0.0, 0.0, 0.0, 0.0])

julia> Tables.columntable(DimTable(ds; refdims=(Z(1:2),)))  # manually add new refdims (all columns repeat)
(X = [1, 2, 3, 4, 5, 1, 2, 3, 4, 5  …  1, 2, 3, 4, 5, 1, 2, 3, 4, 5], Y = [1, 1, 1, 1, 1, 2, 2, 2, 2, 2  …  2, 2, 2, 2, 2, 3, 3, 3, 3, 3], Z = [1, 1, 1, 1, 1, 1, 1, 1, 1, 1  …  2, 2, 2, 2, 2, 2, 2, 2, 2, 2], x = [1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0  …  1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0], y = [0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0  …  0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0])

[codecov[bot]]

Codecov Report

:x: Patch coverage is 96.00000% with 2 lines in your changes missing coverage. Please review. :white_check_mark: Project coverage is 82.78%. Comparing base (2e5a812) to head (67d3f0f). :warning: Report is 7 commits behind head on breaking.

Files with missing lines	Patch %	Lines
ext/DimensionalDataAlgebraOfGraphicsExt.jl	66.66%	1 Missing :warning:
src/tables.jl	97.87%	1 Missing :warning:

Additional details and impacted files

@@             Coverage Diff              @@
##           breaking    #1119      +/-   ##
============================================
+ Coverage     82.43%   82.78%   +0.34%     
============================================
  Files            57       57              
  Lines          5647     5668      +21     
============================================
+ Hits           4655     4692      +37     
+ Misses          992      976      -16     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

:rocket: New features to boost your workflow:

• :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[rafaqz]

@sethaxen the conflicts against breaking are pretty extreme, sorry about that.

But we should try getting it merged there for the next breaking release (which is very soon)

[sethaxen]

@sethaxen the conflicts against breaking are pretty extreme, sorry about that.

But we should try getting it merged there for the next breaking release (which is very soon)

Currently the default behavior is to not add refdims to columns. What makes this breaking? And, if this will be considered breaking anyways, should I then restore the default behavior being to include refdims in columns?

[rafaqz]

Oh right I misunderstood. So yes its fine to merge to main.

But: then there will be the other problem that I cant easily do the merge because neither side of the Tables.jl changes are my code, and its a lot.

If I merge to main can you commit to merging main into breaking soon afterwards? I will make sure its only your changes to merge.

[sethaxen]

Oh right I misunderstood. So yes its fine to merge to main.

But: then there will be the other problem that I cant easily do the merge because neither side of the Tables.jl changes are my code, and its a lot.

If I merge to main can you commit to merging main into breaking soon afterwards? I will make sure its only your changes to merge.

Oh I see what you mean. Oof, both PRs rewrite the tables tests. But most of the changes to tests in this PR are I think it will actually be easier to redo this PR onto breaking. Okay with me opening a new PR against breaking with these changes (with default being to include refdims as columns)?

[rafaqz]

Yes changing the default would be fine on breaking.

But this PR is already moved to breaking so no need for a new one.