search

giannimonaco / flowAI

3 stars 6 forks source link

Bug in subsetting just the goodCellIDs #16

Open llrs opened 1 year ago

llrs commented 1 year ago

In flow_auto_qc the call to https://github.com/giannimonaco/flowAI/blob/2af15bdabc0416e9e63882cea6f6fd16557e0872/R/auto-qc.R#L313 might fail.

1) If goodCellIDs is integer(0) because sub_exprs[goodCellIDs, ] is a matrix of 0 rows. 2) If it is just an event (for example goodCellIDs == 1) because the matrix is converted to a numeric vector (is.null(dim(sub_exprs[1, ]))).

Both cases stop with errors "The actual number of cells in data section (1) is not consistent with keyword '$TOT' (0)" and "Argument 'exprs' must be numeric matrix with colnames attribute set".

These problems might bee in other functions and be behind https://github.com/giannimonaco/flowAI/issues/4: not a single event got through the QC checks (which I find very surprising and might require further look into the data).

This can be triggered using the default parameters with a bad file.

I can submit a patch if you wish (note that there have been changes in the git repository of Bioconductor with the branch rename). My proposed solution is: 1) Provide an informative error message or warning 2) Add drop = FALSE in the call sub_expres[goodCellIDs, , drop = FALSE].

giannimonaco commented 1 year ago

Hi! Thank you for spotting this issue! I like your proposed solutions to add "drop = FALSE" for when the value is 1 and an informative message when it is 0. If you can, please go ahead making the changes.

Thank you for warning me about the branch rename too. I just renamed the main github branch name from master to devel.

Best wishes, Gianni

On Mon, Mar 27, 2023 at 2:46 PM Lluís Revilla @.***> wrote:

In flow_auto_qc the call to https://github.com/giannimonaco/flowAI/blob/2af15bdabc0416e9e63882cea6f6fd16557e0872/R/auto-qc.R#L313 might fail.

  1. If goodCellIDs is integer(0) because sub_exprs[goodCellIDs, ] is a matrix of 0 rows.
  2. If it is just an event (for example goodCellIDs == 1) because the matrix is converted to a numeric vector (is.null(dim(sub_exprs[1, ]))).

Both cases stop with errors "The actual number of cells in data section (1) is not consistent with keyword '$TOT' (0)" and "Argument 'exprs' must be numeric matrix with colnames attribute set".

These problems might bee in other functions and be behind #4 https://github.com/giannimonaco/flowAI/issues/4: not a single event got through the QC checks (which I find very surprising and might require further look into the data).

This can be triggered using the default parameters with a bad file.

I can submit a patch if you wish (note that there have been changes in the git repository of Bioconductor with the branch rename https://support.bioconductor.org/p/9149923/#9150043). My proposed solution is:

  1. Provide an informative error message or warning
  2. Add drop = FALSE in the call sub_expres[goodCellIDs, , drop = FALSE] .

— Reply to this email directly, view it on GitHub https://github.com/giannimonaco/flowAI/issues/16, or unsubscribe https://github.com/notifications/unsubscribe-auth/AC2UTEGZPOVJALN2FRJH2M3W6GD2DANCNFSM6AAAAAAWJDO57I . You are receiving this because you are subscribed to this thread.Message ID: @.***>