rawDiag

[cpanse]

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

• Repository: https://github.com/cpanse/rawDiag

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

[x] I understand that by submitting my package to Bioconductor,
the package source and all review commentary are visible to the
general public.

[x] I have read the Bioconductor [Package Submission](https://bioconductor.org/developers/package-submission/)
instructions. My package is consistent with the Bioconductor
[Package Guidelines](https://contributions.bioconductor.org/).

[x] I understand Bioconductor [Package Naming Policy](https://bioconductor.org/developers/package-submission/#naming) and acknowledge
Bioconductor may retain use of package name.

[x ] I understand that a minimum requirement for package acceptance
is to pass R CMD check and R CMD BiocCheck with no ERROR or WARNINGS.
Passing these checks does not result in automatic acceptance. The
package will then undergo a formal review and recommendations for
acceptance regarding other Bioconductor standards will be addressed.

[ x] My package addresses statistical or bioinformatic issues related
to the analysis and comprehension of high throughput omics data.

[ x] I am committed to the long-term maintenance of my package. This
includes monitoring the [support site](https://support.bioconductor.org/) for issues that users may
have, subscribing to the [bioc-devel](https://stat.ethz.ch/mailman/listinfo/bioc-devel) mailing list to stay aware
of developments in the Bioconductor community, responding promptly
to requests for updates from the Core team in response to changes in
R or underlying software.

[x ] I am familiar with the [Bioconductor code of conduct](https://bioconductor.org/about/code-of-conduct/) and
agree to abide by it.

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

[x] The 'devel' branch for new packages and features.
[x] The stable 'release' branch, made available every six
months, for bug fixes.
[x] Bioconductor version control using [Git](http://bioconductor.org/developers/how-to/git/)
(optionally [via GitHub](http://bioconductor.org/developers/how-to/git/sync-existing-repositories/)).

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]

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: "WARNINGS, 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.3 LTS): rawDiag_0.99.19.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/rawDiag to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

[bioc-issue-bot]

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

[bioc-issue-bot]

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/rawDiag to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

[bioc-issue-bot]

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

[bioc-issue-bot]

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: "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 Single Package Builder: Linux (Ubuntu 22.04.3 LTS): rawDiag_0.99.21.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/rawDiag to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

[bioc-issue-bot]

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

[bioc-issue-bot]

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: "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 Single Package Builder: Linux (Ubuntu 22.04.3 LTS): rawDiag_0.99.22.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/rawDiag to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

[bioc-issue-bot]

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

[bioc-issue-bot]

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.3 LTS): rawDiag_0.99.23.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/rawDiag to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

[cpanse]

Dear @lazappi; thanks for your work. I really appreciate it.

Please find my comments (Version: 0.99.23) below:

Hi @cpanse

Thanks for submitting rawDiag 🎉! Below is my review of your package. Please reply here if anything is unclear or needs any further explanation.

What next?

Please address the comments as best as you can. When you are ready for me to check the package again please reply to let me know with a summary of changes you have made or any other responses. You can use the "Quote reply" option in the ... menu on this comment to reply directly to my points below.

Luke

Review

Key: 🚨 Required ⚠️ Recommended 🟢 Optional ❓ Question

General package development

* [X]  ⚠️ Please address notes from `R CMD build`

seems to be fine

* [X]  ⚠️ Please address notes and warnings from `BiocCheck::BiocCheck()`

Zero warnings; notes are in progress. BiocCheck::BiocCheck() is very sensitive

  * Specifically the `WARNING` about using `.Deprecated`

The package has been used for a while (2017) see github.com/fgcz/rawDiag so I have to deal with the situation.

* [X]  ⚠️ Please address notes from `R CMD check`

fix that

  * The check failed for me locally. If it passes on the build system it's probably fine though.

NAMESPACE file

* [X]  🚨 Please rename `read.raw` to be consistent with the other functions

done. its renamed to readRaw

The NEWS file

* [X]  🟢 Remember to update the NEWS file when you make changes to the package

done

Documentation

Vignette

* [X]  🚨 Please add an _Introduction_ section. This should:

  * [X]  Introduce your package
  * [x]  Include motivation for inclusion in Bioconductor
  * [x]  Include comparison to similar packages (if applicable)

* [X]  🚨 Please add an _Installation_ section that includes:

  * [x]  Bioconductor installation instructions (not evaluated)
  * [x]  Instructions for installing additional system dependencies

* [ ]  ⚠️ The vignette is very brief, I think it can be expanded to better explain the package and its functionality

* [x]  ⚠️ In general `echo = TRUE` is preferred in vignettes to show the code that is being run

* [x]  🚨 Please remove commented code that isn't run

* [ ]  🟢 I got an error when I tried to run the plotting chunk:
  ```r
  Error in `ggplot()`:
  ! `mapping` should be created with `aes()`.
  ✖ You've supplied a <character> object
  ```

I can not reproduce the error.

Man pages

* [ ]  ⚠️ It is recommended to add a package man page

It is a great idea; I am worried that I duplicate my vignette text. I am happy to write a package man page if you have a good package as a template example.

* [ ]  🟢 I noticed some unrendered code in the documentation, you might want to check what the rendered output looks like

Unit tests

* [x]  ⚠️ The `context()` function is superseded in recent **{testthat}** versions

* [x]  ⚠️ The tests can be better organised into files for different parts of the package, usually they match the structure of the `R/` directory

* [ ]  ⚠️ The current tests are minimal, please consider adding more tests

work in progress.

Code

R

* [x]  🚨 Using `paste` in `message()`, `warning()`, `stop()` is not needed

done as much as possible

* [ ]  ⚠️ Consider adding checks that function arguments are valid

I added issue: https://github.com/cpanse/rawDiag/issues/1 will follow up later

  * For several of the function you could easily use `match.arg`

* [x]  🚨 Parallelisation should use the `BiocParallel` package

done

* [x]  🚨 Please remove commented code that isn't used

should be done

* [x]  ⚠️ The name of the `shiny()` function is very generation, I would recommend changing it to something more specific to the package

I refactored the shiny application as documented here https://contributions.bioconductor.org/shiny.html

* [x]  ⚠️ I'm not sure about the need for the `appDir` argument in the `shiny()` function, I don't think a user would ever need to set this

I removed the parameter appDir. Good point.

* [ ]  ⚠️ It might be better not to have a default value for the `rawDir` argument in the `shiny()` function. The default value is unlikely to work for most users and making it required would be clearer.

I have to think about it.

* [x]  🚨 Please give the `.f()` function a more descriptive name

removed the code

Third-party code

* [x]  🚨 Please check the inclusion of third-party code follows the [guidelines](https://contributions.bioconductor.org/third-party-code.html)

This is a crucial point. Luckily, it is solved and handled by the https://bioconductor.org/packages/rawrr/ package.

C

[lazappi]

* [X]  ⚠️ Please address notes and warnings from `BiocCheck::BiocCheck()`

Zero warnings; notes are in progress. BiocCheck::BiocCheck() is very sensitive

You are right, it is quite sensitive and some things about line lengths etc. are ok to ignore but there are others that would be easy to address.

  * Specifically the `WARNING` about using `.Deprecated`

The package has been used for a while (2017) see github.com/fgcz/rawDiag so I have to deal with the situation.

It looks like this warning is gone now so I guess you addressed it somehow?

Documentation

Vignette

* [X]  🚨 Please add an _Introduction_ section. This should:

  * [X]  Introduce your package
  * [x]  Include motivation for inclusion in Bioconductor
  * [x]  Include comparison to similar packages (if applicable)

This is great, thanks!

• [X] 🚨 Please add an Installation section that includes:

  • [x] Bioconductor installation instructions (not evaluated)
  • [x] Instructions for installing additional system dependencies

The instructions for the dependencies are great but you are still missing the instructions for installing the package itself with {BiocManager}.

• [ ] ⚠️ The vignette is very brief, I think it can be expanded to better explain the package and its functionality

The vignette still needs more description of the functionality. The rendered vignette shows all the plots but there isn't any information about how to use/interpret them. It would also be good to describe the Shiny app.

Also, I think at least one plot gave a message about a dependency not being installed. Please make sure all the required packages are listed somewhere in DESCRIPTION.

• [x] ⚠️ In general echo = TRUE is preferred in vignettes to show the code that is being run

echo = FALSE is still set on some chunks

• [x] 🚨 Please remove commented code that isn't run

There is still commented code in the vignette

• [ ] 🟢 I got an error when I tried to run the plotting chunk:

  Error in `ggplot()`:
  ! `mapping` should be created with `aes()`.
  ✖ You've supplied a <character> object

I can not reproduce the error.

This worked fine now, not sure what the issue was

Man pages

* [ ]  ⚠️ It is recommended to add a package man page

It is a great idea; I am worried that I duplicate my vignette text. I am happy to write a package man page if you have a good package as a template example.

You can use usethis::use_package_doc() to create an automatically generated one but it's not a big deal so up to you.

* [ ]  🟢 I noticed some unrendered code in the documentation, you might want to check what the rendered output looks like

This is still there in the references section for readRaw()

• [ ] ⚠️ The current tests are minimal, please consider adding more tests

work in progress.

👍🏻 It would be great if you could add more

Code

R

* [x]  🚨 Using `paste` in `message()`, `warning()`, `stop()` is not needed

done as much as possible

I think this could be removed in the the message in the vignette

• [x] 🚨 Parallelisation should use the BiocParallel package

done

Thanks! Usually I would say there should be a BPPARAM argument that lets the user specify how parallel processing is done but I'm not sure how that would work inside a Shiny app.

* [ ]  ⚠️ It might be better not to have a default value for the `rawDir` argument in the `shiny()` function. The default value is unlikely to work for most users and making it required would be clearer.

I have to think about it.

Up to you but I do think it would avoid issues for uses and it's not much extra work for people to need to set the directory

[bioc-issue-bot]

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

[bioc-issue-bot]

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.3 LTS): rawDiag_0.99.24.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/rawDiag to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

[bioc-issue-bot]

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

[bioc-issue-bot]

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.3 LTS): rawDiag_0.99.25.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/rawDiag to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

[bioc-issue-bot]

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

[bioc-issue-bot]

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.3 LTS): rawDiag_0.99.26.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/rawDiag to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

[cpanse]

Dear @lazappi ;

thanks again for you feedback.

* [X]  ⚠️ Please address notes and warnings from `BiocCheck::BiocCheck()`

Zero warnings; notes are in progress. BiocCheck::BiocCheck() is very sensitive

You are right, it is quite sensitive and some things about line lengths etc. are ok to ignore but there are others that would be easy to address.

  * Specifically the `WARNING` about using `.Deprecated`

The package has been used for a while (2017) see github.com/fgcz/rawDiag so I have to deal with the situation.

It looks like this warning is gone now so I guess you addressed it somehow? I added .Deprecated to Rbuildignore.

Documentation

Vignette

* [X]  🚨 Please add an _Introduction_ section. This should:

  * [X]  Introduce your package
  * [x]  Include motivation for inclusion in Bioconductor
  * [x]  Include comparison to similar packages (if applicable)

This is great, thanks!

• [x] 🚨 Please add an Installation section that includes:

  • [x] Bioconductor installation instructions (not evaluated)
  • [x] Instructions for installing additional system dependencies

The instructions for the dependencies are great but you are still missing the instructions for installing the package itself with {BiocManager}.

Added a paragraph (assuming it is going to be on Bioconductor).

• [ ] ⚠️ The vignette is very brief, I think it can be expanded to better explain the package and its functionality

The vignette still needs more description of the functionality. The rendered vignette shows all the plots but there isn't any information about how to use/interpret them. It would also be good to describe the Shiny app.

I added an more text and an other example data set.

Also, I think at least one plot gave a message about a dependency not being installed. Please make sure all the required packages are listed somewhere in DESCRIPTION.

should be the case.

• [x] ⚠️ In general echo = TRUE is preferred in vignettes to show the code that is being run

echo = FALSE is still set on some chunks

• [x] 🚨 Please remove commented code that isn't run

There is still commented code in the vignette

OK; found and removed it. I double checked. I hope it is fixed.

• [ ] 🟢 I got an error when I tried to run the plotting chunk:

  Error in `ggplot()`:
  ! `mapping` should be created with `aes()`.
  ✖ You've supplied a <character> object

I can not reproduce the error. ack.

This worked fine now, not sure what the issue was

Man pages

* [ ]  ⚠️ It is recommended to add a package man page

It is a great idea; I am worried that I duplicate my vignette text. I am happy to write a package man page if you have a good package as a template example.

You can use usethis::use_package_doc() to create an automatically generated one but it's not a big deal so up to you.

OK; I know how to technical do that but it needs to be filled with content easy to maintain.

* [ ]  🟢 I noticed some unrendered code in the documentation, you might want to check what the rendered output looks like

This is still there in the references section for readRaw()

• [ ] ⚠️ The current tests are minimal, please consider adding more tests

This seems to be a bug in RStudio. It looks great in my Linux|Mac terminal. However, using the latest roxygen2 (7.3.1), RStudio renders the reference the right way.

work in progress.

👍🏻 It would be great if you could add more

I added some shinyModule test cases. Testing the visual output failed.

Code

R

* [x]  🚨 Using `paste` in `message()`, `warning()`, `stop()` is not needed

done as much as possible

I think this could be removed in the the message in the vignette

I found it and removed it. tnx.

• [x] 🚨 Parallelisation should use the BiocParallel package

done

Thanks! Usually I would say there should be a BPPARAM argument that lets the user specify how parallel processing is done but I'm not sure how that would work inside a Shiny app.

Yes; I have the same thoughts.

* [ ]  ⚠️ It might be better not to have a default value for the `rawDir` argument in the `shiny()` function. The default value is unlikely to work for most users and making it required would be clearer.

I have to think about it.

Up to you but I do think it would avoid issues for uses and it's not much extra work for people to need to set the directory

Now I got it. Good point; I changed the default to the rawrr sample directory, which should always work for every user. Also, I added the ~/Download folder example to the man page.

Thanks again and best wishes, Christian

[lazappi]

Also, I think at least one plot gave a message about a dependency not being installed. Please make sure all the required packages are listed somewhere in DESCRIPTION.

should be the case.

I still see this warning in the vignette:

## Warning: Computation failed in `stat_binhex()`.
## Caused by error in `compute_group()`:
## ! The package "hexbin" is required for `stat_bin_hex()`.

• [ ] ⚠️ It is recommended to add a package man page

It is a great idea; I am worried that I duplicate my vignette text. I am happy to write a package man page if you have a good package as a template example.

You can use usethis::use_package_doc() to create an automatically generated one but it's not a big deal so up to you.

OK; I know how to technical do that but it needs to be filled with content easy to maintain.

The function I suggested creates a special file that automatically generates the package man page when the documentation is built (mostly from the DESCRIPTION). So other than running the function once you don't have to do/maintain anything. Just wanted to clarify, but still totally up to you.

* [ ]  🟢 I noticed some unrendered code in the documentation, you might want to check what the rendered output looks like

This is still there in the references section for readRaw()

• [ ] ⚠️ The current tests are minimal, please consider adding more tests

This seems to be a bug in RStudio. It looks great in my Linux|Mac terminal. However, using the latest roxygen2 (7.3.1), RStudio renders the reference the right way.

Uh, ok. That's fine then.

• [x] 🚨 Parallelisation should use the BiocParallel package

done

Thanks! Usually I would say there should be a BPPARAM argument that lets the user specify how parallel processing is done but I'm not sure how that would work inside a Shiny app.

Yes; I have the same thoughts.

I guess maybe the user could pass it to the function that creates the app? Or maybe it is worth pointing to the {BiocParallel} docs which explain how to register a backend that you then use in the app?

I think it would be great if you can follow up on these last points but I don't think any of them are important enough to hold up the package so I will approve what you have now.

[lazappi]

Congratulations on getting {rawDiag} into Bioconductor :tada:! It can take a few days to be picked up by the build system but then it should be available in Bioconductor devel and the next release.

[bioc-issue-bot]

cannot build unless issue is open and has the 'pre-review' label or '2. review in progress' label, or is closed and has the 'TESTING' label.

[lshep]

The default branch of your GitHub repository has been added to Bioconductor's git repository as branch devel.

To use the git.bioconductor.org repository, we need an 'ssh' key to associate with your github user name. If your GitHub account already has ssh public keys (https://github.com/cpanse.keys is not empty), then no further steps are required. Otherwise, do the following:

1. Add an SSH key to your github account
2. Submit your SSH key to Bioconductor

See further instructions at

https://bioconductor.org/developers/how-to/git/

for working with this repository. See especially

https://bioconductor.org/developers/how-to/git/new-package-workflow/ https://bioconductor.org/developers/how-to/git/sync-existing-repositories/

to keep your GitHub and Bioconductor repositories in sync.

Your package will be included in the next nigthly 'devel' build (check-out from git at about 6 pm Eastern; build completion around 2pm Eastern the next day) at

https://bioconductor.org/checkResults/

(Builds sometimes fail, so ensure that the date stamps on the main landing page are consistent with the addition of your package). Once the package builds successfully, you package will be available for download in the 'Devel' version of Bioconductor using BiocManager::install("rawDiag"). The package 'landing page' will be created at

https://bioconductor.org/packages/rawDiag

If you have any questions, please contact the bioc-devel mailing list (https://stat.ethz.ch/mailman/listinfo/bioc-devel); this issue will not be monitored further.