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

raneftables returning NamedTuple of DictTables #634

Merged
merged 9 commits into from
Apr 12, 2023
Merged

raneftables returning NamedTuple of DictTables #634

merged 9 commits into from
Apr 12, 2023

Conversation

dmbates
Copy link
Collaborator

@dmbates dmbates commented Aug 11, 2022

  • This is the first stage exploration of returning tabular representations for some of the MixedModel properties through TypedTables.jl
  • Change the result of raneftables to be a NamedTuple of DictTables
    • An individual table gives an informative and attractive printed representation
    • Printing the NamedTuple of DictTables is messy and confusing
    • Extracting properties from the DictTable can be awkward b/c the column name "(Intercept)", purposely chosen to be difficult to represent as a Symbol, must be written as var"(Intercept)" or an equivalent

For example,

julia> slm01 = let
           form = @formula reaction ~ 1 + days + (1+days|subj)
           fit(MixedModel, form, sleep; contrasts)
       end;

julia> raneftables(slm01)  # confusing display
(subj = {"S308" = (subj = "S308", var"(Intercept)" = 2.8158188821572576, days = 9.075511758123813), "S309" = (subj = "S309", var"(Intercept)" = -40.04844169634357, days = -8.644079444480187), "S310" = (subj = "S310", var"(Intercept)" = -38.43306372090656, days = -5.513397995849837), "S330" = (subj = "S330", var"(Intercept)" = 22.832112155574148, days = -4.658717421575887), "S331" = (subj = "S331", var"(Intercept)" = 21.549840457094, days = -2.944492928895566), "S332" = (subj = "S332", var"(Intercept)" = 8.815541388546052, days = -0.23520071393588618), "S333" = (subj = "S333", var"(Intercept)" = 16.441907858079695, days = -0.15880872279991523), "S334" = (subj = "S334", var"(Intercept)" = -6.996670702793803, days = 1.0327272102714635), "S335" = (subj = "S335", var"(Intercept)" = -1.0375872924983427, days = -10.599415818014403), "S337" = (subj = "S337", var"(Intercept)" = 34.66629369085548, days = 8.6323845720063), },)

julia> retbl = only(raneftables(slm01))  # cleaner display
DictTable with 2 columns and 18 rows:
 subj   (Intercept)  days
 ─────┬───────────────────────
 S308 │ 2.81582      9.07551
 S309 │ -40.0484     -8.64408
 S310 │ -38.4331     -5.5134
 S330 │ 22.8321      -4.65872
 S331 │ 21.5498      -2.94449
 S332 │ 8.81554      -0.235201
 S333 │ 16.4419      -0.158809
 S334 │ -6.99667     1.03273
 S335 │ -1.03759     -10.5994
 S337 │ 34.6663      8.63238
 S349 │ -24.558      1.06438
 S350 │ -12.3345     6.47168
 S351 │ 4.274        -2.95533
 S352 │ 20.6222      3.56171
 S369 │ 3.25854      0.871711
 S370 │ -24.7101     4.6597
 S371 │ 0.723262     -0.971053
 S372 │ 12.1189      1.3107

julia> retbl.days     # individual columns are still in Dictionary form
18-element Dictionaries.Dictionary{String, Float64}
 "S308"9.075511758123813
 "S309"-8.644079444480187
 "S310"-5.513397995849837
 "S330"-4.658717421575887
 "S331"-2.944492928895566
 "S332"-0.23520071393588618
 "S333"-0.15880872279991523
 "S334"1.0327272102714635
 "S335"-10.599415818014403
 "S337"8.6323845720063
 "S349"1.0643762563726427
 "S350"6.47167509512872
 "S351"-2.9553318138154885
 "S352"3.561712791627834
 "S369"0.8717108029006246
 "S370"4.659700952698563
 "S371"-0.9710526399373302
 "S372"1.310698060178722

julia> show(retbl.days.values)  # the values property is indeed a vector (and stored as such)
[9.075511758123813, -8.644079444480187, -5.513397995849837, -4.658717421575887, -2.944492928895566, -0.23520071393588618, -0.15880872279991523, 1.0327272102714635, -10.599415818014403, 8.6323845720063, 1.0643762563726427, 6.47167509512872, -2.9553318138154885, 3.561712791627834, 0.8717108029006246, 4.659700952698563, -0.9710526399373302, 1.310698060178722]

@dmbates dmbates marked this pull request as draft August 11, 2022 13:54
@dmbates dmbates requested a review from palday August 11, 2022 13:54
src/mixedmodel.jl Outdated Show resolved Hide resolved
Suggestion from JuliaFormatter

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@dmbates
Copy link
Collaborator Author

dmbates commented Aug 11, 2022

It may be better to return a Vector{DictTable} rather than a NamedTuple{DictTable} because the expression for the grouping factor is the name of the keys in the DictTable.

Update: not really a big win

julia> collect(raneftables(slm01))
1-element Vector{TypedTables.DictTable{String, NamedTuple{(:subj, Symbol("(Intercept)"), :days), Tuple{String, Float64, Float64}}, NamedTuple{(:subj, Symbol("(Intercept)"), :days), Tuple{Dictionaries.Indices{String}, Dictionaries.Dictionary{String, Float64}, Dictionaries.Dictionary{String, Float64}}}, Dictionaries.Indices{String}}}:
 {"S308" = (subj = "S308", var"(Intercept)" = 2.8158188821572576, days = 9.075511758123813), "S309" = (subj = "S309", var"(Intercept)" = -40.04844169634357, days = -8.644079444480187), "S310" = (subj = "S310", var"(Intercept)" = -38.43306372090656, days = -5.513397995849837), "S330" = (subj = "S330", var"(Intercept)" = 22.832112155574148, days = -4.658717421575887), "S331" = (subj = "S331", var"(Intercept)" = 21.549840457094, days = -2.944492928895566), "S332" = (subj = "S332", var"(Intercept)" = 8.815541388546052, days = -0.23520071393588618), "S333" = (subj = "S333", var"(Intercept)" = 16.441907858079695, days = -0.15880872279991523), "S334" = (subj = "S334", var"(Intercept)" = -6.996670702793803, days = 1.0327272102714635), "S335" = (subj = "S335", var"(Intercept)" = -1.0375872924983427, days = -10.599415818014403), "S337" = (subj = "S337", var"(Intercept)" = 34.66629369085548, days = 8.6323845720063), }

@codecov
Copy link

codecov bot commented Aug 11, 2022

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (251ef8a) 95.90% compared to head (a9ba132) 95.90%.

❗ Current head a9ba132 differs from pull request most recent head ff947e4. Consider uploading reports for the commit ff947e4 to get more accurate results

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #634   +/-   ##
=======================================
  Coverage   95.90%   95.90%           
=======================================
  Files          29       29           
  Lines        2732     2733    +1     
=======================================
+ Hits         2620     2621    +1     
  Misses        112      112           
Impacted Files Coverage Δ
src/MixedModels.jl 100.00% <ø> (ø)
src/mixedmodel.jl 91.22% <100.00%> (+0.15%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

src/mixedmodel.jl Outdated Show resolved Hide resolved
@palday
Copy link
Member

palday commented Apr 10, 2023

It may be better to return a Vector{DictTable} rather than a NamedTuple{DictTable} because the expression for the grouping factor is the name of the keys in the DictTable.

Update: not really a big win

julia> collect(raneftables(slm01))
1-element Vector{TypedTables.DictTable{String, NamedTuple{(:subj, Symbol("(Intercept)"), :days), Tuple{String, Float64, Float64}}, NamedTuple{(:subj, Symbol("(Intercept)"), :days), Tuple{Dictionaries.Indices{String}, Dictionaries.Dictionary{String, Float64}, Dictionaries.Dictionary{String, Float64}}}, Dictionaries.Indices{String}}}:
 {"S308" = (subj = "S308", var"(Intercept)" = 2.8158188821572576, days = 9.075511758123813), "S309" = (subj = "S309", var"(Intercept)" = -40.04844169634357, days = -8.644079444480187), "S310" = (subj = "S310", var"(Intercept)" = -38.43306372090656, days = -5.513397995849837), "S330" = (subj = "S330", var"(Intercept)" = 22.832112155574148, days = -4.658717421575887), "S331" = (subj = "S331", var"(Intercept)" = 21.549840457094, days = -2.944492928895566), "S332" = (subj = "S332", var"(Intercept)" = 8.815541388546052, days = -0.23520071393588618), "S333" = (subj = "S333", var"(Intercept)" = 16.441907858079695, days = -0.15880872279991523), "S334" = (subj = "S334", var"(Intercept)" = -6.996670702793803, days = 1.0327272102714635), "S335" = (subj = "S335", var"(Intercept)" = -1.0375872924983427, days = -10.599415818014403), "S337" = (subj = "S337", var"(Intercept)" = 34.66629369085548, days = 8.6323845720063), }

I think changing it a Vector would be breaking, but leaving it as NamedTuple with a different concrete Tables.jl-compliant eltype isn't. I also like being able to index by name.

@palday
Copy link
Member

palday commented Apr 10, 2023

@dmbates if you want to take this out of draft, add NEWS (now at 4.10.0), I think we can go ahead and get this merged.

@dmbates dmbates marked this pull request as ready for review April 12, 2023 16:32
NEWS.md Outdated
Comment on lines 1 to 3
MixedModels v4.10.0 Release Notes
==============================
* `raneftables` returns a `NamedTuple` where the names are the grouping factor names and the values are some `Tables.jl`-compatible type. Currently this type is a `DictTable` from `TypedTables.jl`. [#634]
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
MixedModels v4.10.0 Release Notes
==============================
* `raneftables` returns a `NamedTuple` where the names are the grouping factor names and the values are some `Tables.jl`-compatible type. Currently this type is a `DictTable` from `TypedTables.jl`. [#634]
MixedModels v4.11.0 Release Notes
==============================
* `raneftables` returns a `NamedTuple` where the names are the grouping factor names and the values are some `Tables.jl`-compatible type. Currently this type is a `DictTable` from `TypedTables.jl`. [#634]
MixedModels v4.10.0 Release Notes
==============================

Project.toml Outdated
Copy link
Member

Choose a reason for hiding this comment

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

can you bump the version to 4.11? we had a 4.10 earlier this morning 😄

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Will do. I forgot to warn you that I was getting this PR ready to go.

Copy link
Member

Choose a reason for hiding this comment

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

@dmbates No stress. It's not like we'll run out of version numbers. 😄

@palday
Copy link
Member

palday commented Apr 12, 2023

merge and release when ready!

@dmbates dmbates merged commit 8ae74f7 into main Apr 12, 2023
@dmbates dmbates deleted the TypedTables branch April 12, 2023 17:48
dmbates added a commit that referenced this pull request Apr 25, 2023
dmbates added a commit that referenced this pull request Apr 25, 2023
* undo #634 return Table not DictTable

Co-authored-by: Phillip Alday <[email protected]>
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.

2 participants