Skip to content

Instantly share code, notes, and snippets.

@jerryvig
Last active March 4, 2022 17:56
Show Gist options
  • Save jerryvig/0c9ba23ea28b9419567cd2784d7199a1 to your computer and use it in GitHub Desktop.
Save jerryvig/0c9ba23ea28b9419567cd2784d7199a1 to your computer and use it in GitHub Desktop.
Plan of action for RDIN-618 Coverage Statistics Summary Tickets

Need to simplify this to a series of steps that's easy to follow.

  1. Update the CROP Python backend to return the entire strings for "Percent depth of coverage" and "Mean depth of coverage" but do this only for the report type RT_EXOME_DIAGNOSTIC_V4. For exome_v4, the returned dictionary values for percent_depth_of_coverage and mean_depth_of_coverage will be strings like Proportion of tested genes with high confidence: 99.1% and Mean depth of coverage: 50x.

    For all other exome report types (RT_EXOME_DIAGNOSTIC, RT_EXOME_DIAGNOSTIC_V2, & RT_EXOME_DIAGNOSTIC_V3), the existing behavior, must be preserved in which the percent_depth_of_coverage and mean_depth_of_coverage dict values are still returned as stringified values of only the numbers such as 99.8% and 50x. This is because the report rendering service code will be updated asynchronous to these changes, and we don't control when those rendering service changes are released.

    The relevant code needs updates in at least 2 places.

  2. Update the CROP UI code for the <CoverageStatisticsSummary> React component to inspect the string values for percent_depth_of_coverage and mean_depth_of_coverage that are provided to it such that if those strings already contain their prefixes such as Percent depth of coverage at 20x:, Proportion of tested genes with high confidence:, or Mean depth of coverage:, then display the entire string values for those fields. We've suggested perhaps using a regex to realize this check, but other options are possible such as String.prototype.startsWith(). If the provided values do not have their prefix texts, then continue rendering with the existing behavior in which the prefix text for the numerical values that are hardcoded in the UI as Percent depth of coverage at 20x: and Mean depth of coverage: are used for the rendering.

    See https://github.com/invitae-internal/invitae-crop/blob/develop/crop/app/web/src/components/views/reportDetail/tabs/report/CoverageStatisticsSummary.tsx#L26 and https://github.com/invitae-internal/invitae-crop/blob/develop/crop/app/web/src/components/views/reportDetail/tabs/report/CoverageStatisticsSummary.tsx#L29

  3. Apply the same logic updates made in the CROP UI code to the <CoverageStatisticsSummary> React component in RDIN-618 to the corresponding React component in the rendering service code that's also named <CoverageStatisticsSummary>. Just like on the CROP UI, the rendering service version of this component must also inspect the string values for meanDepthOfCoverage & percentDepthOfCoverage that it's provided with a regular expression or with String.prototype.startsWith() to determine if it needs to render the hardcoded values for the prefixes Percent depth of coverage at 20x: and Mean depth of coverage: or only render the string values coming from the CROP backend that already include the correct coverage summary stats prefixes. The logic to generate these prefixes on the CROP Python backend will be added in RDIN-618 as well.

    The <CoverageStatisticsSummary> React component code is in the rendering-service repo @ https://github.com/invitae-internal/rendering-service/blob/main/src/components/Reports/CoverageStatisticsSummary/CoverageStatisticsSummary.tsx

    For reference the <CoverageStatisticsSummary> React component lives in the CROP UI code at https://github.com/invitae-internal/invitae-crop/blob/develop/crop/app/web/src/components/views/reportDetail/tabs/report/CoverageStatisticsSummary.tsx#L18

  4. OPTIONAL - NEED TO FILE TICKET: After the PDF rendering service updates made to the <CoverageStatisticsSummary> React component in RDIN-748 are released in production, we can return to the CROP backend in report_writer.py and document_writer.py and update _get_coverage_statistics_summary() to always return the full coverage summary statistics texts including the prefixes for percent_depth_of_coverage and mean_depth_of_coverage for all report types. This should work not only for RT_EXOME_DIAGNOSTIC_V4, but for all other report types so that the returned text is now independent of the report_type and always includes the prefix texts such as Percent depth of coverage at 20x:, Mean depth of coverage:, and `. We can also do the reversion of the special logic implemented on the CROP web UI in the same ticket as this one where we handle the reversion of the report type specific logic on the backend.

  5. OPTIONAL - NEED TO FILE JIRA TICKET: After the changes in step 4) are released in production, we can return to both the CROP UI and the rendering service and remove the string inspection logic added in steps 2) and 3) to both versions of the <CoverageStatisticsSummary> React component, so that it always renders the full text it's provided on the percent_depth_of_coverage and mean_depth_of_coverage fields and it no longer uses hardcoded prefixes in the rendering.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment