Repository navigation
refactor: document CCV classification, look up method type by value - #80
Merged
Merged
Conversation
Two cleanups from the #78 review (the duplicate proposition union was already fixed on this branch by "fix dup"): - ccv: give VariantOncogenicityStatement.classification a description so its Field(...) documents the field like every sibling. - validators: resolve the method type by enum VALUE (cls.MethodType(method_type)) instead of by mangled member NAME (cls.MethodType[method_type.upper()]). The name-based lookup only worked because every value equals its lowercased member name; methodType holds the value, so look it up by value. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
korikuzma
approved these changes
Sep 30, 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
2 participants
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.
Two remaining cleanups from the review of #78. (The third — the duplicated
iriReferencemember inExperimentalVariantFunctionalImpactProposition.object— was already fixed onballot-updatesin "fix dup", so it's not included here.)1.
VariantOncogenicityStatement.classificationwas undocumentedThe field used
Field(...,)with nodescription, so theField()wrapper added nothing over a bare annotation and the field shipped undocumented (unlike its ACMG/AAC siblings and the baseStatement.classification). Added a CCV-appropriate description mirroring the sibling profiles.2. Method type resolved by mangled member NAME instead of value
_validate_method_type_evidence_outcomedidcls.MethodType[method_type.upper()], looking the enum up by member name. That only works because everyMethodTypevalue happens to equal its lowercased member name — a coupling that silently breaks the day a value diverges from its identifier.specifiedBy.methodTypeholds the enum value, so it's now resolved withcls.MethodType(method_type)(value lookup), keeping the same friendly error message.Verification
ruff/ruff-formatpre-commit hooks pass.ValueError, which the surroundingexceptstill converts to the friendly message.Note: local dependency versions are stale (vrs 2.0.0 / cat_vrs 0.5.0, no
ga4gh.core.metadata), so the full model suite could not be executed here — CI on the updated deps should confirm.🤖 Generated with Claude Code