feat!: remove params_table; composed_to_table is the single table surface - #348
Merged
seabbs merged 2 commits intoAug 5, 2026
Merged
Conversation
…e table surface BREAKING CHANGE: params_table(d) is removed. composed_to_table(d) is now the only table-reading verb: it returns the full node/attribute/parameter inventory, and the parameter-only view is a filter over it (`filter(row -> row.role == :param, Tables.rows(composed_to_table(d)))`). This folds into the unregistered 0.2.0 breaking window rather than a deprecation cycle. - build_priors and update now both accept a composed_to_table-shaped table directly, filtering to :param rows internally the same way, so build_priors(composed_to_table(tree)) and update(tree, composed_to_table(tree)) work without hand-filtering. - centred_pool_rows (Pool.jl) and required_parameters (varying.jl) migrate to the _ParamSink walk directly, the same zero-extra-work path params_table used internally, so neither regresses to building the full table on their AD-adjacent call sites. - ParamsTable's show header drops the now-dead params_table/composed_to_table branch. - Tests recast the golden parity fixture as an explicit role == :param filter over composed_to_table, verified against an internal _param_rows helper that still runs the parameter-only walk independently (proving the filter reproduces the historical params_table output). Other tests move to composed_to_table directly where row count is not significant, or to _param_rows where a test's assertions depend on the historical five-column shape or exact row count. - Docs and tutorials teach composed_to_table plus the role filter as the parameter-view idiom.
guards and tests Reviewer follow-ups on the #227 params_table removal: - extend the leaf hot-path testitem to exercise required_parameters and centred_pool_rows directly, not just _param_rows - rename the composed_to_table wrapper type from ParamsTable to ComposedTable (breaking, same unregistered 0.2.0 window) - add the missing # Arguments section to composed_to_table's docstring - pin and document the empty-path/root-row convention that required_parameters and centred_pool_rows fall back to for a bare leaf - give build_priors the same up-front Tables.istable/edge/param guard update already has, so a DI-shaped table is refused loudly - dedupe the restated row-structure sentence in NEWS.md and fix an unrewrapped line
Codecov Report❌ Patch coverage is
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Contributor
|
📖 Documentation preview is ready! View the docs for this PR at: https://EpiAware.github.io/ComposedDistributions.jl/previews/PR348/ This preview will be updated automatically when you push new commits. |
Contributor
Try this Pull Request!Option 1: Julia Package ManagerOpen Julia and type: import Pkg
Pkg.activate(temp=true)
Pkg.add(url="https://github.com/EpiAware/ComposedDistributions.jl", rev="feat/227-remove-params-table")
using ComposedDistributionsOption 2: Local CheckoutIf you have the repo locally: git checkout feat/227-remove-params-table
julia --project=. -e "using Pkg; Pkg.instantiate()" |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #343 (retargets to main automatically when it merges). Sam's single-surface decision, folded into the unregistered 0.2.0 window: params_table is removed outright — function, export, docstring. The params view is the documented filter (role == :param) over composed_to_table; build_priors and update accept the full table directly, filtering internally, and build_priors gains the same DI-shape guard update has.
Hot paths did not regress: centred_pool_rows and required_parameters run the parameter-only sink walk directly, and the CountingLeaf test now pins those exact callers (not a lookalike helper). Both incidentally gained the leaf support their docstrings already claimed — the bare-leaf empty-path convention is pinned by tests and documented. The wrapper type is renamed ParamsTable -> ComposedTable to match the surface. DI's migration is its own to schedule (DI#96); unregistered DI absorbs the break at CD 0.2.0 registration.
Gates: test-fast 2070/2070, quality 242/242; docs rendering stages verified clean for the rename (the only build failure was the known external 429 linkcheck, EpiAwarePackageTools#378). Closes #227's removal decision thread.
This was opened by a bot. Please ping @seabbs for any questions.