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

Missing exports() breaks dependencies. #98

Closed
dansmith01 opened this issue Nov 3, 2021 · 4 comments
Closed

Missing exports() breaks dependencies. #98

dansmith01 opened this issue Nov 3, 2021 · 4 comments
Assignees

Comments

@dansmith01
Copy link

dansmith01 commented Nov 3, 2021

CRAN just notified me they'll be pulling my rbiom package due to:
Error: 'h5writeAttribute.character' is not an exported object from 'namespace:rhdf5'

I found it very helpful to manually call the appropriate class function for h5writeAttribute to ensure numeric variables are properly stored in h5 as integers or floats. However, it looks like PR #87 removed a lot of those exported functions.

Any chance those functions can be exported again and a new version pushed to BioConductor before CRAN removes my package on 2021-11-17? I'll attach a pull request. If this isn't feasible, please let me know asap so I can adapt my package's code to compensate instead.

@dansmith01
Copy link
Author

Added PR #99 and #100 for consideration.

@grimbough grimbough self-assigned this Nov 3, 2021
@bthieurmel
Copy link

Hi,

Same for h5writeDataset function : Missing or unexported object: ‘rhdf5::h5writeDataset.array’ with CRAN "ultimatum" on 2021-11-17.

Thanks.

@grimbough
Copy link
Owner

Sorry this has broken your packages. It's annoying CRAN doesn't also use the developmental version of Bioconductor. This change was made months ago to try and find these problems before it was released.

I'm a bit conflicted here. The function pre-date my time as maintainer of rhdf5, but I've always assumed they were intended as S3 methods because of the .integer etc suffixes, and it feels wrong to export them as regular functions. It looks like in the past they were exported as both regular functions and S3 methods

rhdf5/NAMESPACE

Lines 316 to 366 in 30d6cb7

export(h5writeDataset)
export(h5writeDataset.data.frame)
export(h5writeDataset.list)
export(h5writeDataset.matrix)
export(h5writeDataset.integer)
export(h5writeDataset.double)
export(h5writeDataset.logical)
export(h5writeDataset.character)
export(h5writeDataset.array)
export(h5writeAttribute)
## export(h5writeAttribute.data.frame)
## export(h5writeAttribute.list)
export(h5writeAttribute.matrix)
export(h5writeAttribute.integer)
export(h5writeAttribute.double)
export(h5writeAttribute.logical)
export(h5writeAttribute.character)
export(h5writeAttribute.array)
export(h5delete)
## export(listHandles)
export(h5listIdentifier)
export(h5validObjects)
export(h5version)
export(h5set_extent)
export(h5errorHandling)
export(h5closeAll)
export(h5testFileLocking)
export(h5disableFileLocking)
export(h5enableFileLocking)
## export S3 methods
S3method(h5write, default)
S3method(h5writeDataset, data.frame)
S3method(h5writeDataset, list)
S3method(h5writeDataset, array)
S3method(h5writeDataset, matrix)
S3method(h5writeDataset, integer)
S3method(h5writeDataset, double)
S3method(h5writeDataset, logical)
S3method(h5writeDataset, character)
S3method(h5writeAttribute, default)
S3method(h5writeAttribute, array)
S3method(h5writeAttribute, matrix)
S3method(h5writeAttribute, integer)
S3method(h5writeAttribute, double)
S3method(h5writeAttribute, logical)
S3method(h5writeAttribute, character)

I've also had other users point out that they rely on the S3 method export e.g. #89 and I suspect #99 & #100 will break it their usecase again. I don't know if I can force Roxygen to provide both types of export.

Ultimately I think the whole thing is a bit over engineered, because all the h5writeDataset and h5writeAttribute variants end up using the .array version of the function, and the choice of of HDF5 datatype is decided inside that function based on the type of the R object you're passing. I'm not sure you're actually gaining anything by using the explicit versions of the functions. I think this code demonstrates how the R type overrides the choice of method:

library(rhdf5)

some_numerics <- as.numeric(1:10)
is(some_numerics)
#> [1] "numeric" "vector"

## create a file and try to write our numeric as an integer
tf <- tempfile()
fid <- H5Fcreate(name = tf)
rhdf5:::h5writeDataset.integer( some_numerics, h5loc = fid, name = "still_a_numeric" )
rhdf5:::h5writeDataset.integer( as.integer(some_numerics), h5loc = fid, name = "now_an_integer" )
H5Fclose(fid)

## check the datatype of the new datasets
h5ls(tf)
#>   group            name       otype  dclass dim
#> 0     /  now_an_integer H5I_DATASET INTEGER  10
#> 1     / still_a_numeric H5I_DATASET   FLOAT  10

If you have an example where this isn't the behaviour I'm happy to keep looking at this, but I think it might be cleaner (and not actually change how anything works) for your code to use h5writeDataset() and h5writeAttribute() without the suffixes.

@dansmith01
Copy link
Author

@grimbough good point. I went ahead and closed the PRs and will adapt the code on my end. By the way, thank you for maintaining rhdf5!

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

3 participants