-
Notifications
You must be signed in to change notification settings - Fork 7
Feature/persistent student id best match file #157
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
jayckaiser
wants to merge
27
commits into
main
Choose a base branch
from
feature/persistent_student_id_best_match_file
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
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
…//github.com/edanalytics/earthmover_edfi_bundles into feature/student_id_wrapper_require_rows_exit_code__human_readable_student_id_match_file
… always output id_no_student_id_match file.
…sure it is always output.
…ws early-exit at end.
…one record is always returned in best_id_match.
…the join with rosters always fails.
…ed rate; Turn off require_rows failures to move this logic into the EM DAG.
…g different outputs on successful and unsuccessful runs.
Fix minor spelling error.
3 tasks
Remove match rate from output file (as this only reflects the very first run).
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.
Feature: Persistent Student ID Best Match File
Description & motivation
In the current version of the
student_idbundle, if an assessment source file does not hit theREQUIRED_ID_MATCH_RATEthreshold, Earthmover fails and so non-matched student output file is generated.This PR makes the following updates:
PR Merge Priority:
This (and all dependent repo branches) are required for the new round of assessment loading in South Carolina. I can keep these branches pinned for the moment, but eventually they should be merged to main. Note that because we pin branches directly in the
earthmover_packages.yamlfile in our implementation repos, it is not essential for this dependency to be merged before other repo PRs.Changes to existing files:
packages/student_ids/earthmover.yaml: Incorporate changes outlined above.New files created:
packages/student_ids/best_id_match.txtt: Create a new text template for describing subsequent partner action for resolving non-matched students.packages/student_ids/no_match.csv: A one-line no-match default to ensure non-matched students are not dropped.Tests and QC done:
This branch has been tested extensively in South Carolina Test and QA.
Future ToDos & Questions:
best_id_match.txtttemplate may be too verbose for the generic package. Perhaps it should be simplified here and implementation-specific variations should be composed.no_match.csvfile should be replaced with an in-line source, once we incorporate Earthmover PR131.