Feature/dim developer org type - #15
Merged
Merged
Conversation
Extends `_transform_delimited_dimension` so a dimension built from a delimited string can attach columns from a secondary silver table via a name match. Backwards-compatible: existing configs without an `enrich_from` block behave exactly as before. The hook is reached by the new sentinel source expression `"ENRICHED"` in `schema_map` — it returns the attached value (or `None` when no secondary row matches). Where multiple secondary rows share the same match key the first occurrence wins; for the upcoming `dim_developer` case this is analytically safe because `vin_developers.org_type` for a given `org_name` always comes from the same `accounts` row.
Three extra unit tests lock in the behaviour the original suite left implicit: - "first wins" actually verified with two different values (the earlier test had identical values, so a last-wins refactor would have slipped through), - a NULL `match_target` value on a secondary-table row is skipped rather than treated as a valid lookup key, - an empty secondary table yields all-NULL enrichment without raising (realistic during a cold run before silver populates vin_developers). A defensive guard now raises `ValueError` upfront when the `enrich_from` config block is missing one of `table`, `match_target`, or `attach`, instead of failing deep inside the transform with a bare `KeyError`. Three more tests cover that.
`dim_developer` now carries a nullable `org_type` column populated from `silver.vin_developers.org_type` (which is itself joined from `accounts.vin_organisationtype` and cleaned of unresolved numeric codes in the bronze→silver step). Match is by `developer_name` ↔ `org_name`, not `accountid`, because `dim_developer` is still sourced from the delimited string in `vin_candidates.developersaggregated` — roughly 5% of names there have no `accounts` row at all, and dropping them would orphan `bridge_candidate_developer`. Those names get `org_type = NULL`, which the slide-in design accepts. The non-trivial trade-off (name-keyed instead of FK-keyed) is described inline on the config block and recapped in `transformations/developers.py` for readers who never see the spec.
Smoke check on the freshly built gold DB: - the column exists, - at least a few hundred rows carry a non-NULL org_type, - no row leaks a literal numeric Dataverse code (e.g. "5001") that silver's `transform_developers` was supposed to null out. This guards both the new enrichment wiring and the existing silver cleanup against a future change that accidentally bypasses it.
The downstream consumer (`dim_developer.org_type` in gold, added in the same change set) name-matches into `silver.vin_developers.org_type` and would surface a literal "5001" verbatim if this cleanup ever got removed or moved. Spell that out in-place so a future maintainer deleting "dead code" sees the contract before deleting it. Also leaves a pointer for the eventual `accountid`-keyed outrigger restructure — the place this cleanup belongs once `dim_developer` references `dim_organization` directly.
The previous form `dev[numeric_like.fillna(False)]` is equivalent today because `_read_table` returns a default RangeIndex, but it breaks silently if the helper is ever changed to set a non-trivial index. Align with the plan's `.loc` + `.dropna` form so the index alignment is explicit and the assertion stays robust.
Purely cosmetic — line-length, brace placement, and parenthesization tweaks across the three files touched by this branch. No behaviour change; `ruff check` was already clean before this commit.
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
1 participant
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.
No description provided.