method documentation & vignette revision

[HelenaLC]

• vignette revision according as discussed at stemangiola/tidyseurat#66, including
  • loading full set dependencies in the preamble
  • fixing vignette HTML rendering on Bioc (see here)
  • hyperlinking external packages using BiocStyle::CRAN/Biocpkg() and friends
  • consistent style to improve readability and distinguish between, e.g.,
    ecosystems (tidyverse), classes/variables (tibble), functions (unnest())
  • prettify table summarizing tidySingleCellExperiment's functionality
• method documentation revision as proposed at stemangiola/tidyseurat#66, including
  • inheriting man pages from parent dplyr, tidyr, ..., methods
  • minor fixed for consistent style and ordering of roxygen headers
  • added attach.R and zzz.R from stemangiola/tidySingleCellExperiment#69 to assure needed dependencies are attached
• outstanding pain points:
  • rather than attach.R and zzz.R, would depending (instead of importing) packages be an option? how to other tidy packages deal with defining for S3 methods for foreign generics?
  • dplyr's bind_rows/cols() masks the ttservice version; currently
    need to write ttservice::bind_rows/cols() for methods to work
• the good news:
  • R CMD check is currently clean
  • devtools::document() is warning-free

[stemangiola]

outstanding pain points:

• rather than attach.R and zzz.R, would depending (instead of importing) packages be an option? how to other tidy packages deal with defining for S3 methods for foreign generics?

I took those files from the tidyverse package. Would add them to depend load those package analogously to what attach.R does?

Maybe changing this could be another pull request. I am never against simplifying stuff ;)

• dplyr's bind_rows/cols() masks the ttservice version; currently need to write ttservice::bind_rows/cols() for methods to work

Yes I'm not sure why bind_* have not been developed as methods. I would prefer using tidySingleCellExperiment:: because ttservice should be completely hidden from the user.

If only tidySingleCellExperiment is loaded or loaded last, this is not a problem. However, I understand that this explicit declaration makes the example more robust.

[stemangiola]

the good news:

• R CMD check is currently clean
• devtools::document() is warning-free

Amazing!

[HelenaLC]

For 1. I think this is could be due to how ttservice documents them, but will doublecheck. I.e., we get what we inherit, so fixing it upstream might resolve it.

2.-4. Agreed & I can fix those. Re depending vs. attaching - I am not sure. I‘ll try to give it a shirt and see first hand. Plus do some research. Let‘s see!

Re bind_rows etc., still worth thinking about how we can resolve this. Eg, as a user, I get pretty confused with non-informative errors because some other method was dispatched :/

[stemangiola]

For 1. I think this is could be due to how ttservice documents them, but will doublecheck. I.e., we get what we inherit, so fixing it upstream might resolve it.

Let's think about this after this PR then.

2.-4. Agreed & I can fix those.

When these are done, most likely the PR is good to go!

Looking forward to accept it.

(also please have a look if they pass the github actions, before requiring code review)

[HelenaLC]

Hm. I wonder if these fails could be related to older R/Bioc versions in the GHA? I could not reproduce them on R 4.3/Bioc 3.17, so am a bit stuck how to fix this.

[stemangiola]

Hm. I wonder if these fails could be related to older R/Bioc versions in the GHA? I could not reproduce them on R 4.3/Bioc 3.17, so am a bit stuck how to fix this.

I see just one warning

Warning: declared S3 method 'rename.SingleCellExperiment' not found

must be some little bug, I don't think it depends on versions of R/Bioc

[HelenaLC]

Hm. I wonder if these fails could be related to older R/Bioc versions in the GHA? I could not reproduce them on R 4.3/Bioc 3.17, so am a bit stuck how to fix this.

I see just one warning

Warning: declared S3 method 'rename.SingleCellExperiment' not found

must be some little bug, I don't think it depends on versions of R/Bioc

Yeah, I think you're right - hoping this'll fix it! Maybe worth considering to eventually up GHA versions in any case, just to be sure we're testing in the same realm as Bioc servers will (as to not get unexpected build/check warnings/errors when pushing upstream).

[stemangiola]

Hm. I wonder if these fails could be related to older R/Bioc versions in the GHA? I could not reproduce them on R 4.3/Bioc 3.17, so am a bit stuck how to fix this.

I see just one warning

Warning: declared S3 method 'rename.SingleCellExperiment' not found

must be some little bug, I don't think it depends on versions of R/Bioc

Yeah, I think you're right - hoping this'll fix it! Maybe worth considering to eventually up GHA versions in any case, just to be sure we're testing in the same realm as Bioc servers will (as to not get unexpected build/check warnings/errors when pushing upstream).

To have quicker iterations you could even PR onto your master, test, and then push your master into mine after it passes the tests.

The warning is because it thinks, for example, we are extending stats::filter rather than dplyr::filter

https://github.com/stemangiola/tidySingleCellExperiment/actions/runs/5702449898/job/15455712253?pr=74#step:20:143

[HelenaLC]

Replaced by stemangiola/tidySingleCellExperiment#75