Skip sample metadata addition to colData if cellhash is present

[allyhawkins]

When running the multiplexed samples through the workflow, I was getting the following error:

Command error:
  Warning message:
  replacing previous import ‘S4Arrays::makeNindexFromArrayViewport’ by ‘DelayedArray::makeNindexFromArrayViewport’ when loading ‘SummarizedExperiment’ 
  Error in scpcaTools::metadata_to_coldata(sce, join_columns = "library_id") : 
    The specified `join_columns` are producing multiple matches, but only one match is allowed.
  Calls: format_czi -> <Anonymous>
  Execution halted

This is because we are using the sce_to_anndata.R script to convert all objects to AnnData prior to running CellAssign. In a separate process, we use this same script to do the conversion to produce the final AnnData output. However, in that process, we are skipping any multiplexed samples and when running CellAssign, we are keeping multiplexed samples. The issue here is that we have a step in the format_czi function that merges the colData with the sample_metadata using library_id as the join column. This won't work when trying to convert an SCE that contains multiplexed data since the library_id will match up with multiple samples.

To get around this, I added a check in the format_czi function to make sure that cellhash is not an altExp found in the SCE object. If so, no sample metadata is added to the colData. I also updated the check for converting altExp objects to only convert if the feature name was not cellhash.

[jashapiro]

Looks good overall, but I suggested a tiny restructure to reduce nesting and I think you have the test backward?

[jashapiro]

I think you want this condition first, still, then use } else if (opt$feature_name == "cellhash") { to reduce nesting depth.

[jashapiro]

we should be consistent in whether we use warning() or message() here. I guess I would lean toward warning() for both?

[allyhawkins]

I updated the order of the if statement and then used warning for both warning messages. This should be ready for another look.

[jashapiro]

LGTM, with one spacing update

[jashapiro]

Oh wait, this else is another else... we need to fix that

[jashapiro]

nvm, this is fine, it is the nested if! (This is what I was saying about failing first... just helps my brain parse it)