Executor: Fix check for unmatched student IDs #6
Conversation
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
tomreitz
approved these changes
Jan 13, 2026
Collaborator
tomreitz
left a comment
tomreitz
left a comment
There was a problem hiding this comment.
Approving this @johncmerfeld, what you said it does makes sense. Thanks!
| self.highest_match_id_name = highest_match["source_column_name"] | ||
| self.highest_match_id_type = highest_match["edfi_column_name"] | ||
| self.num_unmatched_students = int(highest_match["num_rows"]) - int(highest_match["num_matches"]) | ||
|
|
Collaborator
There was a problem hiding this comment.
(Feel free to take or leave this comment - it's not introduced by this change, just something I noticed while reviewing it.)
The sorting of match_rates could be done by earthmover, rather than here. The best_id_match transformation already does; id_match_rates could as well.
johncmerfeld
added a commit
that referenced
this pull request
Jan 14, 2026
johncmerfeld
added a commit
that referenced
this pull request
Jan 14, 2026
johncmerfeld
added a commit
that referenced
this pull request
Jan 14, 2026
johncmerfeld
added a commit
that referenced
this pull request
Jan 14, 2026
Merged
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.
Tested by running a CIRCLE file (with updated code) with 100% match rate and confirming that the executor does not wrongly report that there are unmatched students, and that it still reports the correct number of unmatched students otherwise
For a bundle such as CIRCLE or TX_KEA whose input has multiple header rows, the executor’s current approach to checking whether Earthmover produced unmatched students (“does the file contain non-header rows?”) is too brittle.
count_unmatched_studentsfunctionhandle_unmatched_studentsinto two functions:record_highest_match_rateandreport_unmatched_studentsrecord_highest_match_rateruns afterearthmover run, whether the run succeeds or not. It reads the match rates CSV and records information about the highest match rate found in that file. If there are no much unmatched students, don't upload the (empty but existent) unmatched students CSVreport_unmatched_studentsis a pared-down version ofhandle_unmatched_studentsthat simply checks whether a) there are any unmatched students to report and b) whether there are so many that we shouldn't bother sending back the unmatched students CSV