openpipelines-bio / openpipeline

https://openpipelines.bio
MIT License
25 stars 11 forks source link

Fix hvg subsetting bug #385

Closed VladimirShitov closed 11 months ago

VladimirShitov commented 1 year ago

Changelog

This line in scvi integration component is supposed to subset highly-variable genes: adata = adata[:,adata.var['var_input']].copy()

However, this will not happen for the following reasons:

  1. Instead of "var_input" par["var_input"] should be used
  2. Even with proper key, adata.var[key] will return a column from adata.var and not the list of selected genes as expected

This PR fixes the issue

Checklist before requesting a review

DriesSchaumont commented 1 year ago

@VladimirShitov Good catch! Would you be able to implement tests?

VladimirShitov commented 11 months ago

Added test and fixed small comments. The function for HVG subsetting is now moved to a separate utils script, as it is used in other components as well. The problem is that the test pipeline fails because there is "No space left on device", so I'm not 100% sure that importing utils works correctly

image
DriesSchaumont commented 11 months ago

@VladimirShitov The disk issue seemed like a temporary one (?). I am now getting this:

Screenshot 2023-06-05 at 09 33 04
VladimirShitov commented 11 months ago

Yep, this is a problem from my side now 😅 Fixing

VladimirShitov commented 11 months ago

Fixed it :) @DriesSchaumont , can you check?

DriesSchaumont commented 11 months ago

@VladimirShitov Could you adress the comment from Robrecht?

VladimirShitov commented 11 months ago

Done, please check :)

DriesSchaumont commented 11 months ago

LGTM! @rcannood If you would like to have a look