Constituent names, OSS hygiene, and ingest/CI/deps fixes#2
Merged
Conversation
The output is a donor-level table (it already holds constituent_email), so first_name/last_name are carried through when the ConstituentEvent feed supplies them — enabling personalization and golden-record re-joins without touching the numeric columns the estimators consume. Guarded: populated via groupby.first() (skips nulls) when firstName/lastName are present, null otherwise. +2 tests.
…policy) - .github/ISSUE_TEMPLATE: bug + feature templates and a config with docs and UniSchema contact links - .github/PULL_REQUEST_TEMPLATE.md tied to the make ci gate - CODE_OF_CONDUCT.md (Contributor Covenant 2.1), linked from CONTRIBUTING - SECURITY.md with a private disclosure path Lowers first-contribution friction and fills in GitHub's community profile.
flake8 was a dev dependency but unconfigured and never run in CI. Add a .flake8 that enforces the checks that catch actual bugs — pyflakes (F: unused imports/vars, undefined names) and syntax (E9) — instead of formatter-territory whitespace/line-length. Wire it into 'make ci' and ci.yml so it stays green. Clear the 31 violations it surfaced: 25 unused imports and 4 unused locals across philanthropy/ and tests/, plus two root-cause dead-code blocks (an unused input_cols branch in EncounterTransformer.fit, and an unused Pipeline scaffold in a leakage test). No behavior change; full suite still 1162 passed.
…_events
UniSchema writes each event to {prefix}/{vendor}/{yyyy}/{mm}/{dd}/{eventId}.json,
so a non-recursive scan of the top dir returned nothing on a real egress tree
(the docstring's own read_constituent_events("data/egress/") example read zero
events). Walk the tree recursively, skip *.manifest.json batch sidecars, and
sort by relative path for deterministic ordering. Flat directories still work.
classifiers/requires-python advertise 3.9-3.13 but CI only ran 3.10-3.11. Test what we claim to support.
Core install (pipelines, estimators) no longer pulls in a plotting stack. matplotlib/seaborn are lazy-imported inside the plotting functions and declared under [viz] (and kept in [dev] so tests still exercise them).
WealthPercentileTransformer had 'wealth_cols: list[str] | None' with no 'from __future__ import annotations', so Python 3.9 evaluated the PEP-604 union at class-definition and raised TypeError on import. Adding 3.9 to the CI matrix surfaced this pre-existing bug. It was the only module missing the guard.
With matplotlib/seaborn now an optional [viz] extra, test.yml's manual dep install no longer pulled them in and test_visualisation.py failed collection with ModuleNotFoundError. Skip that module via pytest.importorskip when viz is absent, and install .[viz] in the coverage job so the plots stay covered.
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.
Batches the
feat/donor-names-oss-hygienework into main.Constituent names & hygiene (earlier commits)
feat(ingest): carry constituent first/last name into the feature tablechore: community health files (issue/PR templates, CoC, security policy)chore: flake8 lint gate + clear real-defect violationsdocs(changelog): log the aboveIngest / CI / deps fixes (this session)
fix(ingest):read_constituent_eventsnow recurses into UniSchema's date-partitioned egress ({vendor}/{yyyy}/{mm}/{dd}/{eventId}.json), skips*.manifest.jsonsidecars, deterministic sort. +5 tests.ci: expand test matrix to the full advertised range (3.9-3.13).deps: move matplotlib/seaborn to an optional[viz]extra, lazy-imported.Verification
Full suite green locally: 1150 passed, 16 skipped, 89.23% coverage. flake8 clean. Pre-push hook (full suite) passed.