Bioconductor / Contributions

Contribute Packages to Bioconductor
134 stars 33 forks source link

tidySpatialExperiment #3130

Closed william-hutchison closed 8 months ago

william-hutchison commented 1 year ago

Update the following URL to point to the GitHub repository of the package you wish to submit to Bioconductor

Confirm the following by editing each check box to '[x]'

I am familiar with the essential aspects of Bioconductor software management, including:

For questions/help about the submission process, including questions about the output of the automatic reports generated by the SPB (Single Package Builder), please use the #package-submission channel of our Community Slack. Follow the link on the home page of the Bioconductor website to sign up.

bioc-issue-bot commented 1 year ago

Hi @william-hutchison

Thanks for submitting your package. We are taking a quick look at it and you will hear back from us soon.

The DESCRIPTION file for this package is:

Type: Package
Package: tidySpatialExperiment
Title: Brings SpatialExperiment to the tidyverse 
Version: 0.99.0
Authors@R: c(
    person("William", "Hutchison", email = "hutchison.w@wehi.edu.au", role = c("aut", "cre")),
    person("Stefano", "Mangiola", email = "mangiolastefano@gmail.com", role = "aut")
  )
Description: tidySpatialExperiment is an adapter that abstracts the SpatialExperiment container 
    in the form of a tibble and allows data manipulation, plotting and nesting using functions from
    the tidyverse ecosystem.
License: GPL-3
Depends:
    R (>= 4.3.0),
    SingleCellExperiment,
    SpatialExperiment,
    tidySingleCellExperiment,
Imports:
    ttservice,
    SummarizedExperiment,
    BiocGenerics,
    dplyr,
    tibble,
    tidyr,
    ggplot2,
    plotly,
    magrittr,
    rlang,
    purrr,
    lifecycle,
    methods,
    utils,
    S4Vectors,
    tidyselect,
    ellipsis,
    vctrs,
    pillar,
    stringr,
    cli,
    fansi,
    Matrix,
    pkgconfig
Suggests:
    BiocStyle,
    testthat,
    knitr,
    markdown,
    SingleCellSignalR,
    SingleR,
    scater,
    scran,
    tidyHeatmap,
    igraph,
    GGally,
    uwot,
    celldex,
    dittoSeq,
    EnsDb.Hsapiens.v86,
    cowplot, 
    microbenchmark
VignetteBuilder: 
    knitr
Biarch: true
biocViews: AssayDomain, Infrastructure, RNASeq, DifferentialExpression, GeneExpression, Normalization, Clustering, QualityControl, Sequencing
Encoding: UTF-8
LazyData: true
RoxygenNote: 7.2.3
Roxygen: list(markdown = TRUE)
URL: https://github.com/william-hutchison/tidySpatialExperiment
BugReports: https://github.com/william-hutchison/tidySpatialExperiment/issues
vjcitn commented 1 year ago

you need DropletUtils in Suggests, or _R_CHECK_SUGGESTSONLY will lead to error

bioc-issue-bot commented 1 year ago

Your package has been added to git.bioconductor.org to continue the pre-review process. A build report will be posted shortly. Please fix any ERROR and WARNING in the build report before a reviewer is assigned or provide a justification on why you feel the ERROR or WARNING should be granted an exception.

IMPORTANT: Please read this documentation for setting up remotes to push to git.bioconductor.org. All changes should be pushed to git.bioconductor.org moving forward. It is required to push a version bump to git.bioconductor.org to trigger a new build report.

Bioconductor utilized your github ssh-keys for git.bioconductor.org access. To manage keys and future access you may want to active your Bioconductor Git Credentials Account

bioc-issue-bot commented 1 year ago

Dear Package contributor,

This is the automated single package builder at bioconductor.org.

Your package has been built on the Bioconductor Build System.

On one or more platforms, the build results were: "ERROR". This may mean there is a problem with the package that you need to fix. Or it may mean that there is a problem with the build system itself.

Please see the build report for more details.

The following are build products from R CMD build on the Bioconductor Build System: macOS 12.6.5 Monterey: tidySpatialExperiment_0.99.0.tar.gz Linux (Ubuntu 22.04.2 LTS): tidySpatialExperiment_0.99.0.tar.gz

Links above active for 21 days.

Remember: if you submitted your package after July 7th, 2020, when making changes to your repository push to git@git.bioconductor.org:packages/tidySpatialExperiment to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

bioc-issue-bot commented 1 year ago

A reviewer has been assigned to your package for an indepth review. Please respond accordingly to any further comments from the reviewer.

bioc-issue-bot commented 1 year ago

Received a valid push on git.bioconductor.org; starting a build for commit id: 23f040541eae9be46730e92f8dee4e84f01b34d5

bioc-issue-bot commented 1 year ago

Dear Package contributor,

This is the automated single package builder at bioconductor.org.

Your package has been built on the Bioconductor Build System.

On one or more platforms, the build results were: "WARNINGS". This may mean there is a problem with the package that you need to fix. Or it may mean that there is a problem with the build system itself.

Please see the build report for more details.

The following are build products from R CMD build on the Bioconductor Build System: macOS 12.6.5 Monterey: tidySpatialExperiment_0.99.1.tar.gz Linux (Ubuntu 22.04.2 LTS): tidySpatialExperiment_0.99.1.tar.gz

Links above active for 21 days.

Remember: if you submitted your package after July 7th, 2020, when making changes to your repository push to git@git.bioconductor.org:packages/tidySpatialExperiment to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

bioc-issue-bot commented 1 year ago

Received a valid push on git.bioconductor.org; starting a build for commit id: 5733e3b28587ac9662c4aee13ce6c6734fb2e125

bioc-issue-bot commented 1 year ago

Dear Package contributor,

This is the automated single package builder at bioconductor.org.

Your package has been built on the Bioconductor Build System.

Congratulations! The package built without errors or warnings on all platforms.

Please see the build report for more details.

The following are build products from R CMD build on the Bioconductor Build System: Linux (Ubuntu 22.04.2 LTS): tidySpatialExperiment_0.99.2.tar.gz macOS 12.6.5 Monterey: tidySpatialExperiment_0.99.2.tar.gz

Links above active for 21 days.

Remember: if you submitted your package after July 7th, 2020, when making changes to your repository push to git@git.bioconductor.org:packages/tidySpatialExperiment to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

william-hutchison commented 1 year ago

Hello, thank you for your feedback. I have addressed all warnings and errors produced during the automated test process. Let me know if you have any further suggestions

hpages commented 1 year ago

Thanks @william-hutchison. Please consider addressing the following NOTEs:

R CMD check NOTEs:

  1. * checking dependencies in R code ... NOTE
    Packages in Depends field not imported from:
      ‘SpatialExperiment’ ‘tidySingleCellExperiment’
      These packages need to be imported from (in the NAMESPACE file)
      for when this namespace is loaded but not attached.
  2. Consider adding
      importFrom("methods", "callNextMethod", "getMethod", "is")
    to your NAMESPACE file (and ensure that your DESCRIPTION Imports field
    contains 'methods').

    Note that for the above note we actually recommend to import the full methods namespace with import(methods) instead of the suggested selective import.

    Fixing the imports should get rid of most "no visible binding for global variable" and "no visible global function definition" messages. Any remaining one will need to be assessed to determine whether it's a false positive or not.

  3. * checking Rd files ... NOTE
    prepare_Rd: group_by.Rd:70-72: Dropping empty section \seealso

BiocCheck() NOTEs:

  1. * Checking for recommended biocViews...
        * NOTE: Consider adding these automatically suggested biocViews:
          Spatial, Transcriptomics, SingleCell, ImmunoOncology, DataImport

    Maybe not all the terms above are relevant but maybe some of them are?

  2. * Checking vignette directory...
        * NOTE: 'sessionInfo' not found in vignette(s)
  3. * Checking coding practice...
        * NOTE: Avoid using '=' for assignment and use '<-' instead

Let me know if you have any questions.

Thanks, H.

bioc-issue-bot commented 1 year ago

Received a valid push on git.bioconductor.org; starting a build for commit id: abe6e276e93b10ff68f94cf64f1030c4b74cad4c

bioc-issue-bot commented 1 year ago

Dear Package contributor,

This is the automated single package builder at bioconductor.org.

Your package has been built on the Bioconductor Build System.

On one or more platforms, the build results were: "ERROR, skipped". This may mean there is a problem with the package that you need to fix. Or it may mean that there is a problem with the build system itself.

Please see the build report for more details.

The following are build products from R CMD build on the Bioconductor Build System: ERROR before build products produced.

Links above active for 21 days.

Remember: if you submitted your package after July 7th, 2020, when making changes to your repository push to git@git.bioconductor.org:packages/tidySpatialExperiment to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

bioc-issue-bot commented 1 year ago

Received a valid push on git.bioconductor.org; starting a build for commit id: 05b0a894b63e5033aa35b6776c39845a1db8c0df

bioc-issue-bot commented 1 year ago

Dear Package contributor,

This is the automated single package builder at bioconductor.org.

Your package has been built on the Bioconductor Build System.

Congratulations! The package built without errors or warnings on all platforms.

Please see the build report for more details.

The following are build products from R CMD build on the Bioconductor Build System: Linux (Ubuntu 22.04.2 LTS): tidySpatialExperiment_0.99.4.tar.gz macOS 12.6.5 Monterey: tidySpatialExperiment_0.99.4.tar.gz

Links above active for 21 days.

Remember: if you submitted your package after July 7th, 2020, when making changes to your repository push to git@git.bioconductor.org:packages/tidySpatialExperiment to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

william-hutchison commented 1 year ago

Thanks @hpages.

I have addressed most of these notes.

I am not sure if I will be able to silence: Package in Depends field not imported from: tidySingleCellExperiment These packages need to be imported from (in the NAMESPACE file) for when this namespace is loaded but not attached.

The exported functions in tidySingleCellExperiment which I import are themselves imported from other packages within tidySingleCellExperiment, and maintain their original namespace (e.g dplyr), leading to the error Error: object arrange is not exported by 'namespace:tidySingleCellExperiment'.

I am also unable to import the whole of tidySingleCellExperiment as it causes conflicts with ttservice. The other functions I import from tidySingleCellExperiment are not exported.

There are also a few remaining BiocCheck() notes on style, which I will continue to work towards fixing.

Let me know if you have any further suggestions.

William

william-hutchison commented 1 year ago

Although to clarify, I could of course import something which is not needed from tidySingleCellExperiment to silence the note. But that seems like a worse outcome than leaving the note as is.

hpages commented 1 year ago

I am also unable to import the whole of tidySingleCellExperiment as it causes conflicts with ttservice.

I don't see any conflict with ttservice when importing the whole tidySingleCellExperiment. However, I do see a conflict with plotly::plot_ly:

Warning: replacing previous import ‘tidySingleCellExperiment::plot_ly’ by ‘plotly::plot_ly’ when loading ‘tidySpatialExperiment’

At the root of the problem is how tidySummarizedExperiment, tidySingleCellExperiment, and tidySpatialCellExperiment handle the plot_ly generic and methods:

Note that every time a generic is masked by a new generic of the same name, all the methods defined for the masked generic also get masked.

This approach has a lot of problems, too many to discuss them in detail here. Best practice is to define the generic function in one place, and also to define each method only once. Also the default method should just be an alias to the original non-generic function (in this case it should be the plot_ly function defined in the plotly package).

This is easy to do with S4 generics and classes (note that Bioconductor recommends the use of S4 over S3):

  1. Define and export the plot_ly S4 generic and default method:

    setGeneric("plot_ly", signature="data")

    Note that this statement automatically takes care of defining the default method (see it with selectMethod(plot_ly, "ANY")). This should probably go at the bottom of the tidy*Experiment stack, that is, in the tidySummarizedExperiment package. Also tidySummarizedExperiment will still need to import the plot_ly function from the plotly package like it does already.

  2. Each package that wants to define a plot_ly() method must import the plot_ly generic from tidySummarizedExperiment (with importFrom(tidySummarizedExperiment, plot_ly)), then use setMethod() to define the S4 method. E.g. tidySingleCellExperiment will need to do something like:

    setMethod("plot_ly", "SingleCellExperiment",
        function(data = data.frame(), ..., type = NULL, name, color, 
                 colors = NULL, alpha = NULL, stroke, strokes = NULL, alpha_stroke = 1, 
                 size, sizes = c(10, 100), span, spans = c(1, 20), symbol, 
                 symbols = NULL, linetype, linetypes = NULL, split, frame, 
                 width = NULL, height = NULL, source = "A")
        {
            ...
            ...
        }
    )

    Note that the S4 method for SummarizedExperiment objects will (of course) need to be defined in tidySummarizedExperiment, where the S4 generic is defined. So in this case no additional import is needed.

  3. About exports: tidySummarizedExperiment will need to export the plot_ly generic (with export(plot_ly)) and methods (with exportMethods(plot_ly)). Other packages that only define plot_ly S4 methods will need to export these methods (in particular they don't need and should not do export(plot_ly)).

  4. Finally man pages will need to have the corresponding \alias and \usage lines.

Hope this helps, H.

william-hutchison commented 1 year ago

Thank you for your detailed response. I will discuss making these changes in tidySingleCellExperiment and tidySpatialExperiment with @stemangiola, the maintainer of the packages.

stemangiola commented 1 year ago

Thanks @hpages, for the feedback. @william-hutchison we can use for plotly the same strategy that we used for any other methods in tidySCE. If that satisfies the feedback, we can apply it to the other tidy* packages.

bioc-issue-bot commented 1 year ago

Received a valid push on git.bioconductor.org; starting a build for commit id: 7362f2d91def29d117a56f5f07a1a687ab274be8

bioc-issue-bot commented 1 year ago

Dear Package contributor,

This is the automated single package builder at bioconductor.org.

Your package has been built on the Bioconductor Build System.

On one or more platforms, the build results were: "WARNINGS". This may mean there is a problem with the package that you need to fix. Or it may mean that there is a problem with the build system itself.

Please see the build report for more details.

The following are build products from R CMD build on the Bioconductor Build System: Linux (Ubuntu 22.04.2 LTS): tidySpatialExperiment_0.99.5.tar.gz macOS 12.6.5 Monterey: tidySpatialExperiment_0.99.5.tar.gz

Links above active for 21 days.

Remember: if you submitted your package after July 7th, 2020, when making changes to your repository push to git@git.bioconductor.org:packages/tidySpatialExperiment to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

william-hutchison commented 1 year ago

I have prevented the overwriting of plot_ly methods in tidySpatialExperiment. The generic method is now imported from tidySummarizedExperiment. I found that in this instance the tidySingleCellExperiment version of plot_ly is suitable for use with a SpatialExperiment object, so to reduce code duplication I have removed the plot_ly function definition in tidySpatialExperiment.

I have submitted pull requests to tidySingleCellExperiment and tidySummarizedExperiment switching to S4 and preventing the plot_ly methods from being overwritten.

I initially tried to fix these dependencies using S3 to be consistent with the rest of the tidy* packages but was not successful. I think plot_ly is likely an issue because it does not include a generic method, while ggplot ect do. This forces more complicated dependencies, which were difficult to overcome using S3 and may be why the methods were coded to overwrite each other in this way to begin with.

Regardless, I think the changes I submitted to tidySingleCellExperiment and tidySummarizedExperiment have improved the way plot_ly is handled across the tidy* packages. Also I believe they will silence the warning which just popped up once merged.

hpages commented 1 year ago

Thanks for these changes. For your comments about S3 vs S4, I appreciate your point about trying to handle the plot_ly situation in S3, for consistency with the rest of the tidy* packages. And maybe there's an elegant way to do it with S3, I don't know, I'm not familiar enough with S3 to tell. I just know that S4 handles this nicely using the approach I suggested above, which is standard routine in Bioconductor. Thanks again for trying that. We'll see how it goes when the changes in tidySingleCellExperiment and tidySummarizedExperiment get merged and propagate, hopefully soon.

hpages commented 11 months ago

Hi @william-hutchison Any progress on this? The 3.18 release is coming soon and we only have a few days left to add new packages to the 3.18 manifest. Thanks

william-hutchison commented 11 months ago

Hi @hpages, sorry about the delay. A different project has taken my attention over the past week. I will try to get this finished in the next few days, all good if it doesn't get included in the 3.18 release though.

bioc-issue-bot commented 11 months ago

Received a valid push on git.bioconductor.org; starting a build for commit id: 7ff283cb7a703cdd732624529e7361d34c24f024

bioc-issue-bot commented 11 months ago

Received a valid push on git.bioconductor.org; starting a build for commit id: 91e14f500d3dec6fc216bfb19c06752e78c4348f

bioc-issue-bot commented 11 months ago

Dear Package contributor,

This is the automated single package builder at bioconductor.org.

Your package has been built on the Bioconductor Build System.

On one or more platforms, the build results were: "ERROR, WARNINGS". This may mean there is a problem with the package that you need to fix. Or it may mean that there is a problem with the build system itself.

Please see the build report for more details.

The following are build products from R CMD build on the Bioconductor Build System: macOS 12.6.5 Monterey: tidySpatialExperiment_0.99.6.tar.gz Linux (Ubuntu 22.04.2 LTS): tidySpatialExperiment_0.99.6.tar.gz

Links above active for 21 days.

Remember: if you submitted your package after July 7th, 2020, when making changes to your repository push to git@git.bioconductor.org:packages/tidySpatialExperiment to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

bioc-issue-bot commented 11 months ago

Dear Package contributor,

This is the automated single package builder at bioconductor.org.

Your package has been built on the Bioconductor Build System.

On one or more platforms, the build results were: "WARNINGS". This may mean there is a problem with the package that you need to fix. Or it may mean that there is a problem with the build system itself.

Please see the build report for more details.

The following are build products from R CMD build on the Bioconductor Build System: Linux (Ubuntu 22.04.2 LTS): tidySpatialExperiment_0.99.7.tar.gz macOS 12.6.5 Monterey: tidySpatialExperiment_0.99.7.tar.gz

Links above active for 21 days.

Remember: if you submitted your package after July 7th, 2020, when making changes to your repository push to git@git.bioconductor.org:packages/tidySpatialExperiment to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

william-hutchison commented 11 months ago

I believe I have addressed the issues related to ploy_ly. ttservice now contains a S3 generic for plot_ly and a default S3 method. Both are imported by tidySpatialExperiment.

When the user uses plot_ly with the SpatialExperiment class they will be calling the generic in ttservice, which then calls the SpatialExperiment plot_ly method in tidySpatialExperiment, which then extracts the cell data and calls the default plot_ly method in ttservice.

I have also applied this strategy to plot_ly in tidySIngleCellExperiment https://github.com/stemangiola/tidySingleCellExperiment/pull/100 and tidySummarizedExperiment https://github.com/stemangiola/tidySummarizedExperiment/pull/86. If the user has multiple tidy* packages loaded the single generic in ttservice will direct the user's call of plot_ly to the correct method given the class.

In my testing the warning:

Warning: replacing previous import tidySingleCellExperiment::plot_ly by ttservice::plot_ly when loading tidySpatialExperiment

is solved with the newly merged changes to tidySIngleCellExperiment V1.11.8.

I abandoned my previously mentioned implementation of S4 plot_ly sharing between tidy* packages to better match the structure of the ecosystem.

Thanks for your help with this @hpages and let me know if you have any further suggestions.

Kind regards, William

bioc-issue-bot commented 10 months ago

Received a valid push on git.bioconductor.org; starting a build for commit id: 95557549a996919d00059505c49c04b3e7a1842c

bioc-issue-bot commented 10 months ago

Dear Package contributor,

This is the automated single package builder at bioconductor.org.

Your package has been built on the Bioconductor Single Package Builder.

Congratulations! The package built without errors or warnings on all platforms.

Please see the build report for more details.

The following are build products from R CMD build on the Single Package Builder: Linux (Ubuntu 22.04.2 LTS): tidySpatialExperiment_0.99.8.tar.gz macOS 12.7.1 Monterey: tidySpatialExperiment_0.99.8.tar.gz

Links above active for 21 days.

Remember: if you submitted your package after July 7th, 2020, when making changes to your repository push to git@git.bioconductor.org:packages/tidySpatialExperiment to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

william-hutchison commented 10 months ago

Hi @hpages, the last remaining warning was solved by the updated tidySingleCellExperiment being used in the test build.

I will continue to work on tidySpatialExperiment as on ongoing project, but I believe it is now in quite a polished state.

Let me know if you would like to see any more changes before acceptance into Bioconductor and thanks again for your help.

lshep commented 9 months ago

I'm very sorry for the delay; it should not be taking this long. I will check in with @hpages if he plans to revisit this review soon or reassign the reviewer if necessary

hpages commented 9 months ago

Hi @william-hutchison, taking a look now...

hpages commented 9 months ago

@william-hutchison I don't see that the plot_ly situation has been addressed properly e.g. I still see the plot_ly() generic being defined in at least 2 different places, one in ttservice:

library(ttservice)
plot_ly
# function (data = data.frame(), ..., type = NULL, name = NULL, 
#     color = NULL, colors = NULL, alpha = NULL, stroke = NULL, 
#     strokes = NULL, alpha_stroke = 1, size = NULL, sizes = c(10, 
#         100), span = NULL, spans = c(1, 20), symbol = NULL, symbols = NULL, 
#     linetype = NULL, linetypes = NULL, split = NULL, frame = NULL, 
#     width = NULL, height = NULL, source = "A") 
# {
#     UseMethod("plot_ly")
# }
# <bytecode: 0x564201dde750>
# <environment: namespace:ttservice>

and one in tidySummarizedExperiment:

library(tidySummarizedExperiment)
plot_ly
# function (data = data.frame(), ..., type = NULL, name = NULL, 
#     color = NULL, colors = NULL, alpha = NULL, stroke = NULL, 
#     strokes = NULL, alpha_stroke = 1, size = NULL, sizes = c(10, 
#         100), span = NULL, spans = c(1, 20), symbol = NULL, symbols = NULL, 
#     linetype = NULL, linetypes = NULL, split = NULL, frame = NULL, 
#     width = NULL, height = NULL, source = "A") 
# {
#     UseMethod("plot_ly")
# }
# <bytecode: 0x564211f42048>
# <environment: namespace:tidySummarizedExperiment>

FWIW here is what your package stack looks like:

  tidySpatialExperiment
           |
           v
 tidySingleCellExperiment
           |
           v
 tidySummarizedExperiment
           |
           v
       ttservice
           |                           packages that you control
 - - - - - | - - - - - - - - - - - - - - - - - - - - - - - - - - - 
           |                           package that you do not control
           v 
         plotly

As I explained earlier there should be one plot_ly() generic only, defined at the lowest level (i.e. in your ttservice package), and all other packages in your package stack should only define plot_ly() methods.

Best, H.

sessionInfo():

R Under development (unstable) (2024-01-02 r85764)
Platform: x86_64-pc-linux-gnu
Running under: Ubuntu 23.10

Matrix products: default
BLAS:   /home/hpages/R/R-4.4.r85764/lib/libRblas.so 
LAPACK: /usr/lib/x86_64-linux-gnu/lapack/liblapack.so.3.11.0

locale:
 [1] LC_CTYPE=en_US.UTF-8       LC_NUMERIC=C              
 [3] LC_TIME=en_US.UTF-8        LC_COLLATE=en_US.UTF-8    
 [5] LC_MONETARY=en_US.UTF-8    LC_MESSAGES=en_US.UTF-8   
 [7] LC_PAPER=en_US.UTF-8       LC_NAME=C                 
 [9] LC_ADDRESS=C               LC_TELEPHONE=C            
[11] LC_MEASUREMENT=en_US.UTF-8 LC_IDENTIFICATION=C       

time zone: America/Los_Angeles
tzcode source: system (glibc)

attached base packages:
[1] stats4    stats     graphics  grDevices utils     datasets  methods  
[8] base     

other attached packages:
 [1] ggplot2_3.4.4                   tidyr_1.3.0                    
 [3] dplyr_1.1.4                     tidySummarizedExperiment_1.13.0
 [5] SummarizedExperiment_1.33.1     Biobase_2.63.0                 
 [7] GenomicRanges_1.55.1            GenomeInfoDb_1.39.5            
 [9] IRanges_2.37.0                  S4Vectors_0.41.3               
[11] BiocGenerics_0.49.1             MatrixGenerics_1.15.0          
[13] matrixStats_1.2.0               ttservice_0.4.0                

loaded via a namespace (and not attached):
 [1] plotly_4.10.3           utf8_1.2.4              generics_0.1.3         
 [4] SparseArray_1.3.2       bitops_1.0-7            stringi_1.8.3          
 [7] lattice_0.22-5          digest_0.6.33           magrittr_2.0.3         
[10] grid_4.4.0              fastmap_1.1.1           jsonlite_1.8.8         
[13] Matrix_1.6-4            httr_1.4.7              purrr_1.0.2            
[16] fansi_1.0.6             viridisLite_0.4.2       scales_1.3.0           
[19] lazyeval_0.2.2          abind_1.4-5             cli_3.6.2              
[22] rlang_1.1.2             crayon_1.5.2            XVector_0.43.0         
[25] ellipsis_0.3.2          munsell_0.5.0           withr_2.5.2            
[28] DelayedArray_0.29.0     S4Arrays_1.3.1          tools_4.4.0            
[31] colorspace_2.1-0        GenomeInfoDbData_1.2.11 vctrs_0.6.5            
[34] R6_2.5.1                lifecycle_1.0.4         stringr_1.5.1          
[37] zlibbioc_1.49.0         htmlwidgets_1.6.4       pkgconfig_2.0.3        
[40] pillar_1.9.0            gtable_0.3.4            data.table_1.14.10     
[43] glue_1.6.2              tibble_3.2.1            tidyselect_1.2.0       
[46] htmltools_0.5.7         compiler_4.4.0          RCurl_1.98-1.13        
william-hutchison commented 9 months ago

Hi @hpages, thank you for your feedback.

I had removed the plot_ly() generic from tidySummarizedExperiment in our development version but did not push these changes to Bioconductor. Sorry about that, I will address this now.

Kind regards, William

william-hutchison commented 8 months ago

Hi @hpages. The updated tidySummarizedExperiment has been pushed the devel branch of Bioconductor. Thanks again.

hpages commented 8 months ago

Great, thanks!

Last thing on the list: since in the end the plot_ly() generic is defined in the ttservice package, do you really need to list tidySummarizedExperiment in Depends? What is tidySummarizedExperiment used for in tidySpatialExperiment?

If you do need to depend on tidySummarizedExperiment, then make sure to import the package in your NAMESPACE like you did for tidySingleCellExperiment. This issue is actually reported by R CMD check:

* checking dependencies in R code ... NOTE
Package in Depends field not imported from: ‘tidySummarizedExperiment’
  These packages need to be imported from (in the NAMESPACE file)
  for when this namespace is loaded but not attached.

If you don't need to depend on tidySummarizedExperiment then remove it from Depends.

Maybe the same applies to tidySingleCellExperiment? Do you really need to depend on it?

Thanks again, H.

bioc-issue-bot commented 8 months ago

Received a valid push on git.bioconductor.org; starting a build for commit id: f04ba04b09eaf91519ec6dd3b3d2ff88022e67f7

bioc-issue-bot commented 8 months ago

Dear Package contributor,

This is the automated single package builder at bioconductor.org.

Your package has been built on the Bioconductor Single Package Builder.

On one or more platforms, the build results were: "skipped, ERROR". This may mean there is a problem with the package that you need to fix. Or it may mean that there is a problem with the build system itself.

Please see the build report for more details.

The following are build products from R CMD build on the Single Package Builder: ERROR before build products produced.

Links above active for 21 days.

Remember: if you submitted your package after July 7th, 2020, when making changes to your repository push to git@git.bioconductor.org:packages/tidySpatialExperiment to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

bioc-issue-bot commented 8 months ago

Received a valid push on git.bioconductor.org; starting a build for commit id: b6fd6bf220ecea2b49a6646e72bea3f380f226c1

bioc-issue-bot commented 8 months ago

Dear Package contributor,

This is the automated single package builder at bioconductor.org.

Your package has been built on the Bioconductor Single Package Builder.

On one or more platforms, the build results were: "skipped, ERROR". This may mean there is a problem with the package that you need to fix. Or it may mean that there is a problem with the build system itself.

Please see the build report for more details.

The following are build products from R CMD build on the Single Package Builder: ERROR before build products produced.

Links above active for 21 days.

Remember: if you submitted your package after July 7th, 2020, when making changes to your repository push to git@git.bioconductor.org:packages/tidySpatialExperiment to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

bioc-issue-bot commented 8 months ago

Received a valid push on git.bioconductor.org; starting a build for commit id: 7d76d7947d5fb1bbb04d6d84a099a2a399c94d49

bioc-issue-bot commented 8 months ago

Dear Package contributor,

This is the automated single package builder at bioconductor.org.

Your package has been built on the Bioconductor Single Package Builder.

On one or more platforms, the build results were: "skipped, ERROR". This may mean there is a problem with the package that you need to fix. Or it may mean that there is a problem with the build system itself.

Please see the build report for more details.

The following are build products from R CMD build on the Single Package Builder: Linux (Ubuntu 22.04.2 LTS): tidySpatialExperiment_0.99.15.tar.gz

Links above active for 21 days.

Remember: if you submitted your package after July 7th, 2020, when making changes to your repository push to git@git.bioconductor.org:packages/tidySpatialExperiment to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

bioc-issue-bot commented 8 months ago

Received a valid push on git.bioconductor.org; starting a build for commit id: 68c204b4f8382d2e6d9caabdf5d276cd69272674

bioc-issue-bot commented 8 months ago

Dear Package contributor,

This is the automated single package builder at bioconductor.org.

Your package has been built on the Bioconductor Single Package Builder.

On one or more platforms, the build results were: "skipped, ERROR". This may mean there is a problem with the package that you need to fix. Or it may mean that there is a problem with the build system itself.

Please see the build report for more details.

The following are build products from R CMD build on the Single Package Builder: Linux (Ubuntu 22.04.2 LTS): tidySpatialExperiment_0.99.17.tar.gz

Links above active for 21 days.

Remember: if you submitted your package after July 7th, 2020, when making changes to your repository push to git@git.bioconductor.org:packages/tidySpatialExperiment to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

william-hutchison commented 8 months ago

Hi @hpages, thank you for your suggestions.

I have moved tidySummarizedExperiment to Suggests. I believe tidySingleCellExperiment should remain in Depends to attach functions which are not exported by tidySpatialExperiment and are instead called directly from tidySingleCellExperiment e.g count().

Unfortunately I am now encountering an error with the macOS build:

ERROR: dependency SpatialExperiment is not available for package tidySpatialExperiment

All works well on Ubuntu and I am not sure what could be causing this, as SpatialExperiment has always been listed in Depends. I will continue to investigate, but let me know if you have any ideas. Maybe it could be a problem with the build process?

Thanks again

hpages commented 8 months ago

Maybe it was just bad timing. We updated R to the latest R-devel on all our devel build machines a couple of days ago. It could be that the SPB got triggered before all the BioC packages got reinstalled on merida1. Can you please trigger a new build by bumping the version again? Thanks