Skip to content

Conversation

@knapii-developments
Copy link
Contributor

@knapii-developments knapii-developments commented Sep 8, 2020

There was a regression for parsing dense matrices where gene models were only created if the gene had expression data. The PR addresses this issue by creating gene models regardless of the presence of expression data.

There was also a second regression. The feature file in an MTX bundle can have 1 column. As stated by 10X genomics, we now set the gene name to the gene ID in the event 1 column is present.

@codecov
Copy link

codecov bot commented Sep 8, 2020

Codecov Report

Merging #145 into development will increase coverage by 0.17%.
The diff coverage is 100.00%.

Impacted file tree graph

@@               Coverage Diff               @@
##           development     #145      +/-   ##
===============================================
+ Coverage        68.83%   69.00%   +0.17%     
===============================================
  Files               22       22              
  Lines             2663     2678      +15     
===============================================
+ Hits              1833     1848      +15     
  Misses             830      830              
Impacted Files Coverage Δ
ingest/expression_files/dense_ingestor.py 94.02% <100.00%> (-0.05%) ⬇️
ingest/expression_files/expression_files.py 91.26% <100.00%> (+0.63%) ⬆️
ingest/expression_files/mtx.py 94.48% <100.00%> (+0.42%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0e96dd0...6b5b511. Read the comment docs.

@knapii-developments knapii-developments requested review from devonbush and eweitz and removed request for eweitz September 8, 2020 15:38
@knapii-developments knapii-developments changed the title Ea fix dense * SCP2611 Correct gene models for dense matrices Sep 8, 2020
@knapii-developments knapii-developments marked this pull request as ready for review September 8, 2020 15:39
@eweitz eweitz changed the title * SCP2611 Correct gene models for dense matrices * SCP-2611 Correct gene models for dense matrices Sep 8, 2020
Copy link
Contributor

@devonbush devonbush left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks good. Please add a test case that confirms gene models get created for genes with no non-zero expression scores (i.e. a test case that would fail without this fix)

@knapii-developments
Copy link
Contributor Author

Code looks good. Please add a test case that confirms gene models get created for genes with no non-zero expression scores (i.e. a test case that would fail without this fix)

b197446

@knapii-developments knapii-developments changed the title * SCP-2611 Correct gene models for dense matrices * SCP-2611 Correct gene models for dense matrices and account for single column feature files Sep 8, 2020
Copy link
Contributor

@devonbush devonbush left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs a test to cover the single-column gene file case. And one minor code format suggestion

# models maybe less than the batch size.
if len(gene_models) > 0:
self.create_models(
[], [], None, None, gene_models, data_arrays, num_processed, True
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for taking the time to explain this to me, Eno!

@knapii-developments knapii-developments merged commit 6d55525 into development Sep 9, 2020
@knapii-developments knapii-developments deleted the ea-fix-dense branch September 9, 2020 15:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants