Repository navigation
Keep district and district_std columns in output files - #35
Merged
Merged
Conversation
Closed
Copilot
AI
changed the title
[WIP] Fix issue with dropping district column from output files
Keep Jul 14, 2026
district and district_std columns in output files
shauntruelove
approved these changes
Jul 15, 2026
shauntruelove
approved these changes
Jul 15, 2026
…x R CMD check
R CMD check runs all .R files in tests/, but test_modular_functions.R uses
source() with relative paths (e.g. source('R/clean_state_data.R')) which fail
because the working directory during R CMD check is not the package root.
Moving to inst/scripts/ preserves the file for manual use while preventing
R CMD check from executing it automatically.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR aims to ensure district and district_std columns are preserved in the final formatted output (when present) rather than being dropped by column selection logic. It also includes additional robustness changes in school-name matching and a new standalone script for testing modular preprocessing/DQA functions.
Changes:
- Preserve
districtanddistrict_stdduring final formatting output column selection. - Add test coverage for
format_select_columns()to validate retention/absence behavior for district columns. - Harden matching utilities and adjust matching tests; add an
inst/scripts/modular-function test runner script.
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/testthat/test-match_schools_names.R | Tweaks input strings in matching tests (non-exact/unmatched scenarios). |
| tests/testthat/test-final_format_utils.R | Adds unit tests ensuring district columns are kept when present and omitted when absent. |
| R/matching_schools_utils.R | Improves edge-case handling in matching (drop behavior, empty/NULL-safe handling). |
| R/final_format_utils.R | Keeps district and district_std as optional columns in final formatting output. |
| inst/scripts/test_modular_functions.R | Adds a standalone script to exercise modular preprocessing/DQA functions on synthetic data. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
1234
to
1241
| kinder_dat <- format_select_columns( | ||
| kinder_dat, | ||
| required_cols = c("school_id", "year", "school_name", "county_name", | ||
| "enrollment", "current", "med_exempt", "rel_exempt"), | ||
| optional_cols = c("school_type", "school_level", "excluded_note", | ||
| "addr_clean", "city", "zip", "state", "business_status", | ||
| "lat", "lon") | ||
| "lat", "lon", "district", "district_std") | ||
| ) |
Comment on lines
1234
to
1241
| kinder_dat <- format_select_columns( | ||
| kinder_dat, | ||
| required_cols = c("school_id", "year", "school_name", "county_name", | ||
| "enrollment", "current", "med_exempt", "rel_exempt"), | ||
| optional_cols = c("school_type", "school_level", "excluded_note", | ||
| "addr_clean", "city", "zip", "state", "business_status", | ||
| "lat", "lon") | ||
| "lat", "lon", "district", "district_std") | ||
| ) |
Author
There was a problem hiding this comment.
Added district and district_std as "character" entries in expected_classes in commit Add district and district_std to expected_classes in format_fix_column_classes(). These columns will now be coerced to character type consistently with other optional text columns.
…self-contained inline data
shauntruelove
approved these changes
Jul 15, 2026
shauntruelove
approved these changes
Jul 16, 2026
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
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.
The
districtcolumn (added to improve matching accuracy) was being silently dropped from all output files becauseformat_select_columnsfiltered to an explicit allowlist that didn't include it.Changes
R/final_format_utils.R: Add"district"and"district_std"tooptional_colsin theformat_select_columnscall insidegenerate_model_input_tables. Both columns are preserved when present, silently omitted when absent.tests/testthat/test-final_format_utils.R: New test file coveringformat_select_columns— verifies district columns are retained when present and that the function still handles their absence gracefully.