bioRad icon indicating copy to clipboard operation
bioRad copied to clipboard

bind_into_vpts does not always output a list

Open adokter opened this issue 7 years ago • 5 comments

Currently the bind_into_vpts function function outputs a list of vpts class objects when it has as input profiles of different radars, and a single vpts class objects if the profiles are from only one radar. So in the first case the output class is list and in the second case vpts.

I like to keep the functionality to sort your VP files (of potentially different radars) into a vpts for each radar.

Is it undesirable to have different output types depending on the input?

adokter avatar Feb 18 '18 01:02 adokter

@peterdesmet @stijnvanhoey would like some advice here: is it bad to have the class of the output of this function vary depending on the input?

adokter avatar Sep 12 '18 18:09 adokter

Not familiar enough with R to know. @stijnvanhoey @damianooldoni?

peterdesmet avatar Sep 13 '18 09:09 peterdesmet

Typically you can merge lists and single elements using c(). Here below an example based on objects of class integer:

> list1 <-list(2, 3)
> single_element1 <- 4
> merge1 <- c(single_element1, list1)
> merge1
[[1]]
[1] 4

[[2]]
[1] 2

[[3]]
[1] 3

The same holds true with objects of S4 classes (i.e. isS4(variable_name) returns TRUE):

setClass("student", slots=list(name="character", age="numeric", GPA="numeric"))
s4 <- new("student",name="John", age=21, GPA=3.5)
s5 <- new("student",name="Rose", age=23, GPA=4.)
s6 <- new("student",name="Robert", age=20, GPA=2.5)
list3 <- list(s4, s5)
merge3 <- c(s6, list3) 
merge3
[[1]]
An object of class "student"
Slot "name":
[1] "Robert"

Slot "age":
[1] 20

Slot "GPA":
[1] 2.5


[[2]]
An object of class "student"
Slot "name":
[1] "John"

Slot "age":
[1] 21

Slot "GPA":
[1] 3.5


[[3]]
An object of class "student"
Slot "name":
[1] "Rose"

Slot "age":
[1] 23

Slot "GPA":
[1] 4

Be careful:, this doesn't work with S3 classes! Here below a basic example:

> s1 <- list(name = "John", age = 21, GPA = 3.5)
> s2 <- list(name = "Rose", age = 23, GPA = 4.)
> s3 <- list(name = "Robert", age = 20, GPA = 2.5)
> list2 <- list(s1, s2)
> merge2 <- c(s3, list2)
> merge2
$`name`
[1] "Robert"

$age
[1] 20

$GPA
[1] 2.5

[[4]]
[[4]]$`name`
[1] "John"

[[4]]$age
[1] 21

[[4]]$GPA
[1] 3.5


[[5]]
[[5]]$`name`
[1] "Rose"

[[5]]$age
[1] 23

[[5]]$GPA
[1] 4

> length(merge2)
[1] 5

which is wrong! Actually the problem is that S3 classes are a kind of ad hoc "dirty" classes...

damianooldoni avatar Sep 13 '18 10:09 damianooldoni

I would say having different output types (list versus vpts) for a single function is undesirable in general (any programming language), as users will potentially be tricked by it.

A pragmatic solution, taking into account the wish to keep a vpts for each radar, could be to always return a list of vpts, which is a single element in the case of a single radar. Although not optimal (people will slice a 1-element lest when working on a single radar), it integrates with map/apply as such...

stijnvanhoey avatar Sep 22 '18 12:09 stijnvanhoey

To have bind_into_vpts always output a list is practical, but in practice I think most of the times people will use this on the data of a single radar. Users will have to add a lot of slicing, and we will break backward compatibility in a nasty way. Kicking the can down the road to later milestone.

adokter avatar Nov 30 '18 20:11 adokter