flowCore transform method inappropriate for cytoframe

[jacobpwagner]

Currently, calling transform on a cytoframe dispatches to flowCore::transform for a flowFrame based on inheritance. For the case of transformation using a transformList, the result is still correct as exprs<- is eventually called via %on% and exprs<- has a cytoframe override that modifies the underlying data:
https://github.com/RGLab/flowCore/blob/b196fd4ca55ab5093ee61ef03c97abc9fb5eaea1/R/flowFrame-accessors.R#L859
https://github.com/RGLab/flowCore/blob/b196fd4ca55ab5093ee61ef03c97abc9fb5eaea1/R/on-methods.R#L159

flowWorkspace/R/cytoframe.R

Lines 468 to 475 in a779c8c

	setReplaceMethod("exprs",
	signature=signature(object="cytoframe",
	value="matrix"),
	definition=function(object, value)
	{
	cf_setData(object@pointer, value)
	object
	})

For example, this works as expected:

> cf <- load_cytoframe_from_fcs(system.file("extdata", "CytoTrol_CytoTrol_1.fcs", package = "flowWorkspaceData"))
> trans <- linearTransform( a = 10 )
> range(exprs(cf)[, "FSC-H"])
[1]  25008 258223
> transform(cf, transformList('FSC-H', trans))
cytoframe object 'CytoTrol_CytoTrol_1.fcs'
with 119531 cells and 12 observables:
       name         desc   range minRange maxRange
$P1   FSC-A         <NA>  262143        0   262143
$P2   FSC-H         <NA> 2621430        0  2621430
$P3   FSC-W         <NA>  262143        0   262143
$P4   SSC-A         <NA>  262143        0   262143
$P5  B710-A  CD4 PcpCy55  262143     -111   262143
$P6  R660-A     CD38 APC  262143     -111   262143
$P7  R780-A    CD8 APCH7  262143     -111   262143
$P8  V450-A     CD3 V450  262143     -111   262143
$P9  V545-A  HLA-DR V500  262143     -111   262143
$P10 G560-A      CCR7 PE  262143     -111   262143
$P11 G780-A CD45RA PECy7  262143     -111   262143
$P12   Time         <NA>  262143        0   262143
199 keywords are stored in the 'description' slot
> range(exprs(cf)[, "FSC-H"])
[1]  250080 2582230

However, for the inline form (specifying transformations as arguments like `FSC-H`=log(`FSC-H`)), this will fail to appropriately alter the underlying data because it splits off to flowCore:::.transform which instead just returns a new transformed flowFrame:
https://github.com/RGLab/flowCore/blob/b196fd4ca55ab5093ee61ef03c97abc9fb5eaea1/R/flowFrame-accessors.R#L860-L908

For example, this fails to appropriately modify the underlying data:

> cf <- load_cytoframe_from_fcs(system.file("extdata", "CytoTrol_CytoTrol_1.fcs", package = "flowWorkspaceData"))
> range(exprs(cf)[, "FSC-H"])
[1]  25008 258223
> transform(cf, `FSC-H`=log(`FSC-H`))
flowFrame object 'CytoTrol_CytoTrol_1.fcs'
with 119531 cells and 12 observables:
       name         desc  range minRange     maxRange
$P1   FSC-A         <NA> 262143        0 262143.00000
$P2   FSC-H         <NA> 262143     -Inf     12.47665
$P3   FSC-W         <NA> 262143        0 262143.00000
$P4   SSC-A         <NA> 262143        0 262143.00000
$P5  B710-A  CD4 PcpCy55 262143     -111 262143.00000
$P6  R660-A     CD38 APC 262143     -111 262143.00000
$P7  R780-A    CD8 APCH7 262143     -111 262143.00000
$P8  V450-A     CD3 V450 262143     -111 262143.00000
$P9  V545-A  HLA-DR V500 262143     -111 262143.00000
$P10 G560-A      CCR7 PE 262143     -111 262143.00000
$P11 G780-A CD45RA PECy7 262143     -111 262143.00000
$P12   Time         <NA> 262143        0 262143.00000
199 keywords are stored in the 'description' slot
> range(exprs(cf)[, "FSC-H"])
[1]  25008 258223
> range(cf)
     FSC-A  FSC-H  FSC-W  SSC-A B710-A R660-A R780-A V450-A V545-A G560-A G780-A   Time
min      0      0      0      0   -111   -111   -111   -111   -111   -111   -111      0
max 262143 262143 262143 262143 262143 262143 262143 262143 262143 262143 262143 262143

So, probably we need to do one of 2 things:

1. Alter the logic of flowCore::transform to use exprs<- in all cases so the cytoframe-specific exprs<- can take over
2. Add a cytoframe-specific transform method to flowWorkspace so it calls cf_setData directly

[mikejiang]

We want to do 2), because keywords (PnR or flowCore_PnRmax) also needs to be updated properly for cytoframe

[jacobpwagner]

Right, so maybe we should tackle the range issue before this.

[mikejiang]

Maybe we should deprecate the legacy inline form of transformation in flowCore since it is problematic and pretty much has not been heard to be used anywhere else.

[jacobpwagner]

I agree that is an easier solution if you don't think there would be many issues with backwards compatibility. I haven't seen the legacy inline transformation elsewhere, but given the large number of Bioc packages dependent on flowCore, I wouldn't be surprised if there is some usage lurking somewhere in their source. But a full development cycle to deal with the deprecation warnings should be plenty of time.

[SamGG]

If you don't mind, deprecation would be good as I think that I have such inline uses. But if it's easier for you, just break.