Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

base::cbind() #116

Open
multimeric opened this issue May 31, 2024 · 3 comments
Open

base::cbind() #116

multimeric opened this issue May 31, 2024 · 3 comments

Comments

@multimeric
Copy link

When x is an HDF5Matrix, but I haven't attached any BioC packages, I get:

> cbind(x, x)
Error: unable to find an inherited method for functionbindCOLSfor signaturex = "HDF5Matrix"

Seemingly, this is because DelayedArray only implements BiocGenerics::cbind and not base::cbind (no idea how that's even possible though), so calling it directly does work: BiocGenerics::cbind(x, x). library(DelayedArray), which is the more typical way to use this, also works because it overrides base::cbind with BiocGenerics::cbind.

Is there any reason why base::cbind() isn't implemented?

@hpages
Copy link
Contributor

hpages commented Nov 21, 2024

Not too surprising. I wouldn't really expect many things to work properly on S4 objects without having at least one package attached, which is the package where these objects are defined/implemented.

Seemingly, this is because DelayedArray only implements BiocGenerics::cbind and not base::cbind

Well, that's how things work in the S4 world. base::cbind is not an S4 generic function so we define one in the BiocGenerics package. This way we can define cbind() methods for various S4 classes implemented in Bioconductor packages, like the method for DelayedArray objects defined in the DelayedArray package.

Is there any reason why base::cbind() isn't implemented?

base::cbind() is not a generic function so there are no methods associated with it. In other words, there is no way to make base::cbind() work on DelayedArray objects or any S4 object in general. There are good reasons why we have the BiocGenerics package with 100+ S4 generic functions defined in it.

Unfortunately, the solution to your problem is to simply make sure that the DelayedArray package is attached before you start operating on DelayedArray objects or derivatives.

BTW how did you end up with HDF5Matrix object x without having the HDF5Array package attached? Did you load a serialized HDF5Matrix object with readRDS() or load()? Note that serializing out-of-memory objects is generally a bad idea. containsOutOfMemoryData() was introduced in BioC 3.20 to help identify these objects:

> containsOutOfMemoryData(x)
[1] TRUE

Also saveRDS() now issues a warning on these objects.

writeHDF5Array() should be used to save HDF5Array/HDF5Matrix objects. See ?HDF5Array::writeHDF5Array in the HDF5Array package.

Hope this helps,

H.

@multimeric
Copy link
Author

Although it's maybe not a traditional S4 generic, the help for base::cbind does suggest it supports S4 dispatch out of the box.

I typically dislike and avoid attaching packages at all because of all of the problems it causes. In this case my solution was to just use BiocGenerics::cbind instead of cbind in all cases. I assume I got the HDF5Matrix object by using namespaced functions, although I can't exactly remember.

@hpages
Copy link
Contributor

hpages commented Nov 22, 2024

Although it's maybe not a traditional S4 generic, the help for base::cbind does suggest it supports S4 dispatch out of the box.

True, I forgot about that. And IIRC we tried to work directly with base::cbind() in the past but for some reasons decided it was going to be easier to define our own cbind() S4 generics in BiocGenerics. It's a difficult S4 generic because of dispatch on the ellipsis.

I'm looking at the commit history for BiocGenerics and it turns out that cbind() and rbind() were actually the first generics to make it into the package. This was in 2011. So it could be that 13 years later the situation has improved with base::cbind() and that it's now possible to make it work on our S4 objects. I'll try to revisit this when I get some time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants