-
Notifications
You must be signed in to change notification settings - Fork 652
FEAT: Adding pyrit_version to identifiers #1334
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
base: main
Are you sure you want to change the base?
FEAT: Adding pyrit_version to identifiers #1334
Conversation
My more overarching question is when would pyrit_version be used in practice? I figured that our identifiers have enough granular information that a broader |
jsong468
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm! just making sure column is added to our azure sql database and memory integration tests run properly?
In our original
scorer_identifier, I explicitly removedpyrit_versionfrom the identifier because we don't want it as part of the hash (e.g. if thepyrit_versionchanges, we don't want to have to re-calculate scorer metrics).Now that we have more robust general
Identifiers, this PR adds it back.Also added tests and refactored for readability (clearly separating hashes and storage metadata exclusion keys).