Parameterize size and significance metrics, improve robustness (SCP-5241) #321
Conversation
Codecov Report
@@ Coverage Diff @@
## development #321 +/- ##
===============================================
- Coverage 73.77% 73.72% -0.05%
===============================================
Files 31 30 -1
Lines 4129 4141 +12
===============================================
+ Hits 3046 3053 +7
- Misses 1083 1088 +5
|
eweitz
changed the title
bistline
left a comment
bistline
left a comment
There was a problem hiding this comment.
Looks good - do we have associated tickets to update the author DE forms & Rails/Life Sciences API integration to collect and pass these values?
| else: | ||
| delimiter = ',' | ||
| data = pd.read_csv(file_path, delimiter) | ||
| # sep=None invokes detecting separator via csv.Sniffer, per |
Good question -- I'm handling that as part of the ticket for https://github.com/broadinstitute/single_cell_portal_core/pull/1874/files. I just got a basic Core-Ingest integration passing. |
|
Seeing the Codecov complaint makes me think we should apply the same logic from broadinstitute/single_cell_portal_core#1709 to this repository. |
|
Good call -- done! Having the same coverage checks across repos makes sense. |
jlchang
left a comment
jlchang
left a comment
There was a problem hiding this comment.
Manual test performs as described. Minor nit, suggestion to clarify comment for future me.
Approving :)
ingest/author_de.py
Outdated
| comparison_metrics = sorted(comparison_metrics) | ||
|
|
||
| # Put qval first | ||
| # Rank significance 1st (ultimately ranked 2nd) |
There was a problem hiding this comment.
Nit - Was a little confused by the wording... suggesting:
"Arrange significance in expected order (ultimately ranked 2nd)"
ingest/author_de.py
Outdated
| ) | ||
|
|
||
| # Put logfoldchanges first | ||
| # Rank size 1st (ultimately ranked 1st) |
There was a problem hiding this comment.
Nit - to parallel above suggestion (but the original comment was not confusing so use whatever wording feels natural):
"Arrange size in expected order (ultimately ranked first)"
This helps enable authors to specify a wider variety of size and significance metrics for their differential expression data.
It also improves robustness for delimiter detection, and error handling for formats that aren't yet implemented. It integrates with the Core updates over in broadinstitute/single_cell_portal_core#1874.
Automated tests have been updated to verify these changes. To manually test:
This satisfies SCP-5241.