-
Notifications
You must be signed in to change notification settings - Fork 0
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
Feature/rmh brca #67
Feature/rmh brca #67
Conversation
The |
teststatus = record.raw_fields['teststatus'] | ||
if record.raw_fields['variantpathclass'].nil? |
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.
I think you can use safe navigation here
replace
if record.raw_fields['variantpathclass'].nil?
nonpathvarclass = nil
else
nonpathvarclass = record.raw_fields['variantpathclass'].downcase.strip
end
with
nonpathvarclass = record.raw_fields['variantpathclass']&.downcase&.strip
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.
Changes made in commit 1c81ce7. I checked that the variant counts still match.
@@ -29,8 +29,8 @@ class RoyalMarsdenHandler < Import::Germline::ProviderHandler | |||
TEST_TYPE_MAP = { 'affected' => :diagnostic, | |||
'unaffected' => :predictive }.freeze | |||
# rubocop:disable Lint/MixedRegexpCaptureTypes | |||
CDNA_REGEX_PROT = /c\.(?<cdna>.+)(?=(?<separtors>_|;.)p\.(?<impact>.+))/i.freeze | |||
CDNA_REGEX_NOPROT = /c\.(?<cdna>.+)/i.freeze | |||
CDNA_REGEX_PROT = /c\.(?<cdna>.+)(?=(?<separtors>_|;.)p\.(?<impact>.+))/i |
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.
is that supposed to be separtors
?
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.
@lauramccluskey1 should this be separators
?
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.
See commit a7f7fa0- tested and counts are still the same
@@ -45,6 +45,54 @@ def setup | |||
normal_record.raw_fields['teststatus'] = 'NO PATHOGENIC VARIANT IDENTIFIED' | |||
@handler.process_teststatus(@genotype, normal_record) | |||
assert_equal 1, @genotype.attribute_map['teststatus'] | |||
|
|||
non_path_record = build_raw_record('pseudo_id1' => 'bob') | |||
non_path_record.raw_fields['teststatus'] = 'Ex del' |
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.
Can raw_fields
be passed to the build_raw_record
method, eg:
broken_record = build_raw_record('pseudo_id1' => 'bob', raw_fields: { 'teststatus' => 'Ex del' } )
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.
Thanks for the suggestion @ollietulloch ! I have tried that and it doesn't work- I think its because build new record adds the test status field by loading the rawtext_clinical_json. So that needs to be loaded first then we can update the test status afterwards. Let me know if there is another way!
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.
Ah ok, perhaps something for a future PR. It'd be nice for build_raw_record
to be able to popluate raw_fields
Further |
Planio ticket 35297. Updating the Royal Marsden importer to include test status 10.
Variants counts sent as part of QA, and they match the manual counts.