search

PediatricOpenTargets / OpenPedCan-api

2 stars 7 forks source link

Add relapse tumors samples to `/tpm/*` endpoint tables and plots #53

Closed logstar closed 2 years ago

logstar commented 2 years ago

Description

Added relapse tumors samples to /tpm/* endpoint results and plots.

Notes for reviewers:

Type of change

Changed

Added

How Has This Been Tested?

Task link/Screenshot/Terminal returns:

$ ./tests/run_tests.sh 
API base URL: http://localhost:8082
✔ |  OK F W S | Context
✔ | 288       | tests/r_test_scripts/test_endpoint_http.R [592.2 s]                                                                                                                                              

══ Results ═════════════
Duration: 592.3 s

[ FAIL 0 | WARN 0 | SKIP 0 | PASS 288 ]
Done running run_tests.sh

For more details about local testing environment, options, and resources, see README.md of this PR branch.

Checklist

cc @taylordm @chinwallaa

afarrel commented 2 years ago

Hi @logstar , I reviewed your code, tables, and plots. It looks good but before I approve I want to review the title names: eg Pediatric primary and relapsed tumor and GTEx normal adult tissue gene expression. While this is fine, would a more succinct title be Pediatric Tumors and GTEx normal adult tissue gene expression (since you have the primary tumors, relapsed tumors, and normal tissue in the legend right below which can serve two purposes, legend and subtitle.)

What are your thoughts. I'm ready to approve pending this design decision. If the larger group wants the longer descriptive title that's fine.

logstar commented 2 years ago

Hi @logstar , I reviewed your code, tables, and plots. It looks good but before I approve I want to review the title names: eg Pediatric primary and relapsed tumor and GTEx normal adult tissue gene expression. While this is fine, would a more succinct title be Pediatric Tumors and GTEx normal adult tissue gene expression (since you have the primary tumors, relapsed tumors, and normal tissue in the legend right below which can serve two purposes, legend and subtitle.)

What are your thoughts. I'm ready to approve pending this design decision. If the larger group wants the longer descriptive title that's fine.

@afarrel Thank you for reviewing. I think we could keep the longer title, to distinguish primary-and-relapse from primary-only and relapse-only. The title originally uses only "pediatric tumor", but changed to more descriptive ones, which was described in https://github.com/PediatricOpenTargets/OpenPedCan-api/issues/56#issue-1056141024.

logstar commented 2 years ago

Hi @logstar , I reviewed your code, tables, and plots. It looks good. Thanks for your hard work on this.

Hi @afarrel . Thank you for reviewing. I will merge this PR after coordinating with @blackdenc . This PR requires a new database instance to be used for QA site, which needs to be configured by @blackdenc .

kelseykeith commented 1 year ago

Code and Results Review

Everything continues to look good!