Closed ncborcherding closed 2 weeks ago
Hi @ncborcherding
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:
Package: immApex
Title: Tools for Adaptive Immune Receptor Sequence-Based Keras Modeling
Version: 0.99.0
Date: 2024-07-15
Authors@R: c(
person(given = "Nick", family = "Borcherding", role = c("aut", "cre"), email = "ncborch@gmail.com"))
Description: A set of tools to build tensorflow/keras-based models in R from amino acid and nucleotide sequences focusing on adaptive immune receptors. The package includes pre-processing of sequences, unifying gene nomenclature usage, encoding sequences, and combining models. This package will serve as the basis of future immune receptor sequence functions/packages/models compatible with the scRepertoire ecosystem.
License: MIT + file LICENSE
Encoding: UTF-8
LazyData: true
LazyDataCompression: xz
RoxygenNote: 7.3.1
biocViews: Software, ImmunoOncology, SingleCell, Classification, Annotation, Sequencing
Depends:
R (>= 4.0)
Imports: dplyr,
hash,
httr,
keras,
matrixStats,
methods,
rvest,
SingleCellExperiment,
stats,
stringi,
stringr,
tensorflow
Suggests:
BiocStyle,
ggplot2,
knitr,
markdown,
rmarkdown,
scRepertoire,
spelling,
testthat,
viridis
VignetteBuilder: knitr
Language: en-US
URL: https://github.com/ncborcherding/Apex/
BugReports: https://github.com/ncborcherding/Apex/issues
We recommend renaming the vignette to something meaningful (like the name of the package) to avoid naming/loading conflicts.
Is the resource at https://www.imgt.org/ required as a step in your process? If so what is the licensing of this data and use of imgt? If it is different than your package (which it does seem so) and restrictive for use at all this should disclosed to the end user. And make sure your access and use case is permitted
@lshep
Thanks for the feedback -
Cheers
Everything is updated - thanks for the suggestions!
We can move this to the next stage of getting builds but when a reviewer is assigned they will likely look for a message to be printed when the getIMGT function is run about the restrictive license as well.
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
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: macOS 12.7.1 Monterey: immApex_0.99.0.tar.gz Linux (Ubuntu 22.04.3 LTS): immApex_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/immApex
to trigger a new build.
A quick tutorial for setting up remotes and pushing to upstream can be found here.
A reviewer has been assigned to your package for an indepth review. Please respond accordingly to any further comments from the reviewer.
Hi @ncborcherding,
On my Apple M1 MacBook Air, I'm unable to build immApex with its vignettes; see below:
R_ENVIRON_USER=~/.Renviron.bioc R CMD build .
* checking for file ‘./DESCRIPTION’ ... OK
* preparing ‘immApex’:
* checking DESCRIPTION meta-information ... OK
* installing the package to build vignettes
* creating vignettes ... ERROR
--- re-building ‘immApex.Rmd’ using knitr
/Users/peter/.virtualenvs/r-tensorflow/lib/python3.9/site-packages/urllib3/__init__.py:35: NotOpenSSLWarning: urllib3 v2 only supports OpenSSL 1.1.1+, currently the 'ssl' module is compiled with 'LibreSSL 2.8.3'. See: https://github.com/urllib3/urllib3/issues/3020
warnings.warn(
Quitting from lines 105-113 [unnamed-chunk-6] (immApex.Rmd)
Error: processing vignette 'immApex.Rmd' failed with diagnostics:
no applicable method for 'compile' applied to an object of class "c('keras.src.models.functional.Functional', 'keras.models.functional.Functional', 'keras.src.ops.function.Function', 'keras.ops.function.Function', 'keras.src.models.model.Model', 'keras.models.model.Model', 'keras.src.backend.tensorflow.trainer.TensorFlowTrainer', 'keras.backend.tensorflow.trainer.TensorFlowTrainer', 'keras.src.trainers.trainer.Trainer', 'keras.trainers.trainer.Trainer', 'keras.src.layers.layer.Layer', 'keras.layers.layer.Layer', 'keras.src.backend.tensorflow.layer.TFLayer', 'keras.backend.tensorflow.layer.TFLayer', 'keras.src.backend.tensorflow.trackable.KerasAutoTrackable', 'keras.backend.tensorflow.trackable.KerasAutoTrackable', 'tensorflow.python.trackable.autotrackable.AutoTrackable', 'tensorflow.python.trackable.base.Trackable', 'keras.src.ops.operation.Operation', 'keras.ops.operation.Operation', 'keras.src.saving.keras_saveable.KerasSaveable', 'keras.saving.keras_saveable.KerasSaveable', 'python.builtin.object')"
--- failed re-building ‘immApex.Rmd’
SUMMARY: processing the following file failed:
‘immApex.Rmd’
Error: Vignette re-building failed.
Execution halted
Building immApex without vignettes (i.e. ~/.Renviron.bioc R CMD build --no-build-vignettes
), I can narrow the issue down to the first call to variationalSequences()
in the vignette:
library(immApex)
sequences <- generateSequences(prefix.motif = "CAS",
suffix.motif = "YF",
number.of.sequences = 1000,
min.length = 8,
max.length = 16)
variational.sequences <- variationalSequences(sequences,
encoder = "onehotEncoder",
number.of.sequences = 100,
encoder.hidden.dim = c(256, 128),
latent.dim = 16,
batch.size = 16,
call.threshold = 0.1)
#> [1] "Converting to matrix...."
#> [1] "Padding sequences..."
#> [1] "One Hot Encoding sequences..."
#> [1] "Preparing a matrix..."
#> Error in UseMethod("compile"): no applicable method for 'compile' applied to an object of class "c('keras.src.models.functional.Functional', 'keras.models.functional.Functional', 'keras.src.ops.function.Function', 'keras.ops.function.Function', 'keras.src.models.model.Model', 'keras.models.model.Model', 'keras.src.backend.tensorflow.trainer.TensorFlowTrainer', 'keras.backend.tensorflow.trainer.TensorFlowTrainer', 'keras.src.trainers.trainer.Trainer', 'keras.trainers.trainer.Trainer', 'keras.src.layers.layer.Layer', 'keras.layers.layer.Layer', 'keras.src.backend.tensorflow.layer.TFLayer', 'keras.backend.tensorflow.layer.TFLayer', 'keras.src.backend.tensorflow.trackable.KerasAutoTrackable', 'keras.backend.tensorflow.trackable.KerasAutoTrackable', 'tensorflow.python.trackable.autotrackable.AutoTrackable', 'tensorflow.python.trackable.base.Trackable', 'keras.src.ops.operation.Operation', 'keras.ops.operation.Operation', 'keras.src.saving.keras_saveable.KerasSaveable', 'keras.saving.keras_saveable.KerasSaveable', 'python.builtin.object')"
I think my setup is a fairly standard one for Apple Silicon (I installed tensorflow using tensorflow::install_tensorflow()
).
Have you tested immApex on Apple Silicon?
If so, is anything special required to install and/or use it?
Thanks, Pete
@PeteHaitch
Apologies for the delay in response - I wrote the package on Apple Silicon laptop so it is compatible and runs. Let me take a look at your details tomorrow and see if I can troubleshoot.
Thanks, Nick
Thanks. Let me know if there's anything I can do to help
@PeteHaitch
Coincidentally I got a brand new Mac today - so I have a completely fresh install. I was able to replicate the issue:
devtools::check()
══ Documenting ══════════════════════════════════════════════════════════════════
ℹ Updating immApex documentation
ℹ Loading immApex
══ Building ═════════════════════════════════════════════════════════════════════
Setting env vars:
• CFLAGS : -Wall -pedantic -fdiagnostics-color=always
• CXXFLAGS : -Wall -pedantic -fdiagnostics-color=always
• CXX11FLAGS: -Wall -pedantic -fdiagnostics-color=always
• CXX14FLAGS: -Wall -pedantic -fdiagnostics-color=always
• CXX17FLAGS: -Wall -pedantic -fdiagnostics-color=always
• CXX20FLAGS: -Wall -pedantic -fdiagnostics-color=always
── R CMD build ──────────────────────────────────────────────────────────────────
✔ checking for file ‘/Users/borcherding.n/Documents/GitHub/immApex/DESCRIPTION’ ...
─ preparing ‘immApex’:
✔ checking DESCRIPTION meta-information
─ installing the package to build vignettes
E creating vignettes (48.5s)
--- re-building ‘immApex.Rmd’ using knitr
/Users/borcherding.n/.virtualenvs/r-tensorflow/lib/python3.9/site-packages/urllib3/__init__.py:35: NotOpenSSLWarning: urllib3 v2 only supports OpenSSL 1.1.1+, currently the 'ssl' module is compiled with 'LibreSSL 2.8.3'. See: https://github.com/urllib3/urllib3/issues/3020
warnings.warn(
Quitting from lines 105-113 [unnamed-chunk-6] (immApex.Rmd)
Error: processing vignette 'immApex.Rmd' failed with diagnostics:
no applicable method for 'compile' applied to an object of class "c('keras.src.models.functional.Functional', 'keras.models.functional.Functional', 'keras.src.ops.function.Function', 'keras.ops.function.Function', 'keras.src.models.model.Model', 'keras.models.model.Model', 'keras.src.backend.tensorflow.trainer.TensorFlowTrainer', 'keras.backend.tensorflow.trainer.TensorFlowTrainer', 'keras.src.trainers.trainer.Trainer', 'keras.trainers.trainer.Trainer', 'keras.src.layers.layer.Layer', 'keras.layers.layer.Layer', 'keras.src.backend.tensorflow.layer.TFLayer', 'keras.backend.tensorflow.layer.TFLayer', 'keras.src.backend.tensorflow.trackable.KerasAutoTrackable', 'keras.backend.tensorflow.trackable.KerasAutoTrackable', 'tensorflow.python.trackable.autotrackable.AutoTrackable', 'tensorflow.python.trackable.base.Trackable', 'keras.src.ops.operation.Operation', 'keras.ops.operation.Operation', 'keras.src.saving.keras_saveable.KerasSaveable', 'keras.saving.keras_saveable.KerasSaveable', 'python.builtin.object')"
--- failed re-building ‘immApex.Rmd’
SUMMARY: processing the following file failed:
‘immApex.Rmd’
Error: Vignette re-building failed.
Execution halted
Error in `(function (command = NULL, args = character(), error_on_status = TRUE, …`:
! System command 'R' failed
---
Exit status: 1
stdout & stderr: <printed>
---
Type .Last.error to see the more details.
From what I found in several github issues - this seems to be an issue with tensorflow versions and disagreement with keras vs tensorflow.
I was able to solve the problem by just using the keras installer to set up the system (which seems to take care of the version mismatches). I have updated the repo to change the set up instructions for this:
keras::install_keras()
Here are my outputs after making the change:
devtools::check()
══ Documenting ══════════════════════════════════════════════════════════════════
ℹ Updating immApex documentation
ℹ Loading immApex
══ Building ═════════════════════════════════════════════════════════════════════
Setting env vars:
• CFLAGS : -Wall -pedantic -fdiagnostics-color=always
• CXXFLAGS : -Wall -pedantic -fdiagnostics-color=always
• CXX11FLAGS: -Wall -pedantic -fdiagnostics-color=always
• CXX14FLAGS: -Wall -pedantic -fdiagnostics-color=always
• CXX17FLAGS: -Wall -pedantic -fdiagnostics-color=always
• CXX20FLAGS: -Wall -pedantic -fdiagnostics-color=always
── R CMD build ──────────────────────────────────────────────────────────────────
✔ checking for file ‘/Users/borcherding.n/Documents/GitHub/immApex/DESCRIPTION’ ...
─ preparing ‘immApex’:
✔ checking DESCRIPTION meta-information ...
─ installing the package to build vignettes
✔ creating vignettes (51.6s)
─ checking for LF line-endings in source and make files and shell scripts (436ms)
─ checking for empty or unneeded directories
─ building ‘immApex_0.99.0.tar.gz’
══ Checking ═════════════════════════════════════════════════════════════════════
Setting env vars:
• _R_CHECK_CRAN_INCOMING_REMOTE_ : FALSE
• _R_CHECK_CRAN_INCOMING_ : FALSE
• _R_CHECK_FORCE_SUGGESTS_ : FALSE
• _R_CHECK_PACKAGES_USED_IGNORE_UNUSED_IMPORTS_: FALSE
• NOT_CRAN : true
── R CMD check ──────────────────────────────────────────────────────────────────
─ using log directory ‘/private/var/folders/g4/n0vf5jk16jl5b66c82xysg7m0000gp/T/RtmpnRfxeB/file27f2b0bbab7/immApex.Rcheck’
─ using R version 4.4.1 (2024-06-14)
─ using platform: aarch64-apple-darwin20
─ R was compiled by
Apple clang version 14.0.0 (clang-1400.0.29.202)
GNU Fortran (GCC) 12.2.0
─ running under: macOS Sonoma 14.6
─ using session charset: UTF-8
─ using options ‘--no-manual --as-cran’
✔ checking for file ‘immApex/DESCRIPTION’
─ this is package ‘immApex’ version ‘0.99.0’
─ package encoding: UTF-8
✔ checking package namespace information
✔ checking package dependencies (1.1s)
✔ checking if this is a source package ...
✔ checking if there is a namespace
✔ checking for executable files ...
✔ checking for hidden files and directories
✔ checking for portable file names
✔ checking for sufficient/correct file permissions
─ checking whether package ‘immApex’ can be installed ... [22s/22s] OK (22.5s)
N checking installed package size ...
installed size is 5.5Mb
sub-directories of 1Mb or more:
data 5.1Mb
✔ checking package directory ...
✔ checking for future file timestamps (362ms)
✔ checking ‘build’ directory
N checking DESCRIPTION meta-information ...
License stub is invalid DCF.
✔ checking top-level files ...
✔ checking for left-over files ...
✔ checking index information ...
✔ checking package subdirectories (525ms)
✔ checking code files for non-ASCII characters ...
✔ checking R files for syntax errors ...
✔ checking whether the package can be loaded (2.4s)
✔ checking whether the package can be loaded with stated dependencies (2.4s)
✔ checking whether the package can be unloaded cleanly (2.4s)
✔ checking whether the namespace can be loaded with stated dependencies (2.3s)
✔ checking whether the namespace can be unloaded cleanly (2.4s)
✔ checking whether startup messages can be suppressed (4.8s)
✔ checking dependencies in R code (2.4s)
✔ checking S3 generic/method consistency (2.4s)
✔ checking replacement functions (2.4s)
✔ checking foreign function calls (2.4s)
─ checking R code for possible problems ... [11s/12s] OK (12s)
✔ checking Rd files ...
✔ checking Rd metadata ...
✔ checking Rd line widths ...
✔ checking Rd cross-references ...
✔ checking for missing documentation entries (3.2s)
✔ checking for code/documentation mismatches (8s)
✔ checking Rd \usage sections (2.6s)
✔ checking Rd contents ...
✔ checking for unstated dependencies in examples ...
✔ checking contents of ‘data’ directory (704ms)
✔ checking data for non-ASCII characters (1s)
✔ checking LazyData
✔ checking data for ASCII and uncompressed saves ...
✔ checking installed files from ‘inst/doc’ ...
✔ checking files in ‘vignettes’ ...
─ checking examples ... [15s/13s] OK (13.2s)
Examples with CPU (user + system) or elapsed time > 5s
user system elapsed
onehotEncoder 0.921 4.219 0.329
✔ checking for unstated dependencies in ‘tests’ ...
─ checking tests ...
✔ Running ‘spelling.R’
X Comparing ‘spelling.Rout’ to ‘spelling.Rout.save’ ...
6,19c6
< Potential spelling errors:
< WORD FOUND IN
< Repo immApex.Rmd:27
< VAE variationalSequences.Rd:49,51,53,55,62,75
< immApex.Rmd:94,95,96,97,100
< Variational variationalSequences.Rd:5
< de immApex.Rmd:81
< github README.md:37
< novo immApex.Rmd:81
< sequenceDecoder immApex.Rmd:346
< variational variationalSequences.Rd:74
< immApex.Rmd:81
< variationalSequences immApex.Rmd:79,85
< If these are false positive, run `spelling::update_wordlist()`.All Done!
---
> All Done!
[30s/31s] OKthat.R’
* checking for unstated dependencies in vignettes ... OK
* checking package vignettes ... OK
* checking re-building of vignette outputs ... [48s/30s] OK
* checking for non-standard things in the check directory ... OK
* checking for detritus in the temp directory ... NOTE
Found the following files/directories:
‘__autograph_generated_file3w5r58wt.py’
‘__autograph_generated_file7bu2j28d.py’
‘__autograph_generated_file99kbzltt.py’
‘__autograph_generated_file_82w73j9.py’
‘__autograph_generated_fileorh28exo.py’
‘__autograph_generated_fileoxoi_3li.py’
‘__autograph_generated_filerj0h2_s9.py’
‘__autograph_generated_filezx12su1j.py’
* DONE
Status: 3 NOTEs
See
‘/private/var/folders/g4/n0vf5jk16jl5b66c82xysg7m0000gp/T/RtmpnRfxeB/file27f2b0bbab7/immApex.Rcheck/00check.log’
for details.
── R CMD check results ────────────────────────────────────── immApex 0.99.0 ────
Duration: 2m 36.5s
❯ checking installed package size ... NOTE
installed size is 5.5Mb
sub-directories of 1Mb or more:
data 5.1Mb
❯ checking DESCRIPTION meta-information ... NOTE
License stub is invalid DCF.
❯ checking for detritus in the temp directory ... NOTE
Found the following files/directories:
‘__autograph_generated_file3w5r58wt.py’
‘__autograph_generated_file7bu2j28d.py’
‘__autograph_generated_file99kbzltt.py’
‘__autograph_generated_file_82w73j9.py’
‘__autograph_generated_fileorh28exo.py’
‘__autograph_generated_fileoxoi_3li.py’
‘__autograph_generated_filerj0h2_s9.py’
‘__autograph_generated_filezx12su1j.py’
0 errors ✔ | 0 warnings ✔ | 3 notes ✖
Can confirm that after running keras::install_keras()
that R_ENVIRON_USER=~/.Renviron.bioc R CMD build .
was successful.
Thank you for resolving into this.
By the way, don't forget to push any changes, with a version bump, to git@git.bioconductor.org:packages/immApex
to trigger a new build.
I'll start my review of the the package next week.
Sorry for not yet posting my review. I had some unexpected personal stuff to deal with month and I'm now on annual leave until October. I expect that I'll be able to post my review whilst on leave, but please accept my apologies for the delay and slower-than-usual response.
No problem, hope all is well. Thanks for the note.
Hi @ncborcherding,
Thank you for submitting immApex to Bioconductor, and especially for your patience for this review. I've completed my initial checklist review, below, and separated the issues into Required and Recommended points that I would ask you to address before the package can be accepted. Would you please provide line-by-line comments to my initial review so that I know what changes I'm looking for in my re-review.
A reminder when making changes to your repository to bump the version number and push to git@git.bioconductor.org:packages/immApex
to trigger a new build.
Cheers, Pete
getIMGT()
function should probably message the user to warn/remind them about the restrictive license. The corresponding man page should also mention the license and potential restrictions on its use.immapex_gene.list
object (data/immapex_gene.list.rda
) and it's not clear to me that this doesn't violate the 'no derivative work' clause you note.data
and tests/testthat/testdata
. Please try to reduce the size of the included data or explain why you think this cannot be done.% ls -lsh immApex_0.99.0.tar.gz
24984 -rw-r--r--@ 1 peter staff 12M Oct 7 11:20 immApex_0.99.0.tar.gz
% ls -lsh data/
total 4.1M
4.5K -rw-r--r-- 1 hickey lab0605 4.5K Oct 8 10:07 immapex_AA.data.rda
2.5K -rw-r--r-- 1 hickey lab0605 2.5K Oct 8 10:07 immapex_blosum.pam.matrices.rda
4.1M -rw-r--r-- 1 hickey lab0605 4.1M Oct 8 10:07 immapex_example.data.rda
10K -rw-r--r-- 1 hickey lab0605 9.9K Oct 8 10:07 immapex_gene.list.rda
% du -sh tests/testthat/
8.4M tests/testthat/
% du -sh tests/testthat/testdata/*
2.5K tests/testthat/testdata/adjacencyMatrix
4.9M tests/testthat/testdata/formatGenes
13K tests/testthat/testdata/generateSequences
165K tests/testthat/testdata/geometricEncoder
29K tests/testthat/testdata/getIMGT
2.2M tests/testthat/testdata/inferCDR
37K tests/testthat/testdata/mutateSequences
236K tests/testthat/testdata/ohe.encoder
19K tests/testthat/testdata/positional.encoding
5.0K tests/testthat/testdata/probabilityMatrix
721K tests/testthat/testdata/propertyEncoder
23K tests/testthat/testdata/sequenceDecoder
53K tests/testthat/testdata/tokenizeSequences
data/
) requires documentation concerning its creation and source information (see https://contributions.bioconductor.org/data.html#exported-data-and-the-data-directory). The current documentation is pretty sparse, particularly for immapex_example.data
(e.g., the data structure is not described, there's no link to the 10x data).immApex built vignette
immApex manually rendered vignette
keras::install_keras()
rather than tensorflow::install_tensorflow()
.README
R CMD check
flags * checking DESCRIPTION meta-information ... NOTE License stub is invalid DCF.
I think you can fix this by running usethis::use_mit_license()
to correctly specify the MIT licence (see also https://stat.ethz.ch/pipermail/r-package-devel/2024q1/010357.html)set.seed()
(or equivalent) to ensure reproducibility of resultspositionalEncoder
man page appears in the in Details section rather than the Examples section. I think this is just a mistake with your roxygen2 markup%>%
is the only function imported into the NAMESPACE
from dplyr. To reduce the dependencies, either instead import it directly from magrittr (the package that actually defines it) or simply replace %>%
with |>
from basecovr::report()
for a helpful summary report of code coverage to identify areas that may be improved<-
rather than =
for assignment (e.g., es =
is used in vignette)BiocCheck::BiocCheck('immApex_0.99.0.tar.gz')
flags several things to fix (see abbreviated output below)> BiocCheck::BiocCheck('immApex_0.99.0.tar.gz')
ℹ NOTE: 'LazyData:' in the 'DESCRIPTION' should be set to false or removed
ℹ NOTE: Update R version dependency from 4.0 to 4.4.0.
ℹ NOTE: Consider adding the maintainer's ORCID iD in 'Authors@R' with
ℹ NOTE: Avoid sapply(); use vapply()
Found in files:
• R/formatGenes.R (line 104, column 5)
• ...
• R/utils.R (line 49, column 21)
ℹ NOTE: Avoid 1:...; use seq_len() or seq_along()
Found in files:
• adjacencyMatrix.R (line 26, column 63)
• ...
• variationalSequences.R (line 134, column 27)
ℹ NOTE: Avoid 'cat' and 'print' outside of 'show' methods
Found in files:
• print() in R/generateSequences.R (line 36, column 7)
• ...
• print() in R/variationalSequences.R (line 200, column 3)
ℹ NOTE: Avoid using '=' for assignment and use '<-' instead
Found in files:
• R/geometricEncoder.R (line 49, column 15)
• ...
• R/variationalSequences.R (line 88, column 6)
! WARNING: Remove set.seed usage (found 1 times)
• set.seed() in R/variationalSequences.R (line 133, column 3)
! WARNING: Empty or missing \format sections found in data man page(s).
Found in files:
• man/immapex_AA.data.Rd
• ...
• man/immapex_gene.list.Rd
ℹ NOTE: Consider adding runnable examples to man pages that document exported
objects.
• getIR.Rd
• positionalEncoder.Rd
• variationalSequences.Rd
ℹ NOTE: Usage of dontrun / donttest tags found in man page examples. 5% of man
pages use at least one of these tags.
Found in files:
• variationalSequences.Rd
ℹ NOTE: Use donttest instead of dontrun.
Found in files:
• variationalSequences.Rd
ℹ NOTE: Consider adding a NEWS file, so your package news will be included in
Bioconductor release announcements.
── BiocCheck v1.41.17 results ──────────────────────────────────────────────────
✖ 0 ERRORS | ⚠ 2 WARNINGS | • 16 NOTES
ℹ See the immApex.BiocCheck folder and run
browseVignettes(package = 'BiocCheck')
for details.
@PeteHaitch
Thank you for taking the time for the detailed review. Hope all is well on your end. I am slowly chipping away at the list and will let you know when I have adequately addressed them.
Thanks again, Nick
Received a valid push on git.bioconductor.org; starting a build for commit id: e6ff46906471360ca5d1d1899a6f52deeba2cc0f
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/immApex
to trigger a new build.
A quick tutorial for setting up remotes and pushing to upstream can be found here.
@PeteHaitch
Sorry I see I need to track down this build issue, but I think I got the requested changes complete. I am also do not know how you would like me to respond, so I am copying the list you made here:
getIMGT()
function should probably message the user to warn/remind them about the restrictive license. The corresponding man page should also mention the license and potential restrictions on its use.immapex_gene.list
object (data/immapex_gene.list.rda
) and it's not clear to me that this doesn't violate the 'no derivative work' clause you note.
immApex is noncommerical and has appropriate attribution to IMGT. I do not think gene names meets the definition of copy-writable work, which is a standard in order to define derivative work.
data
and tests/testthat/testdata
. Please try to reduce the size of the included data or explain why you think this cannot be done.data/
) requires documentation concerning its creation and source information (see https://contributions.bioconductor.org/data.html#exported-data-and-the-data-directory). The current documentation is pretty sparse, particularly for immapex_example.data
(e.g., the data structure is not described, there's no link to the 10x data).
I have expanded the data and added the links to the original data and links to the explanation of the data formats.
keras::install_keras()
rather than tensorflow::install_tensorflow()
.README
R CMD check
flags * checking DESCRIPTION meta-information ... NOTE License stub is invalid DCF.
I think you can fix this by running usethis::use_mit_license()
to correctly specify the MIT licence (see also https://stat.ethz.ch/pipermail/r-package-devel/2024q1/010357.html)set.seed()
(or equivalent) to ensure reproducibility of resultsI do not know how this would be done before bioconductor accepts the package? Should I add dummy instructions?
- [x] Please add a package-level man page; see https://contributions.bioconductor.org/docs.html#package-level-documentation
I have added a larger introduction section - this package is different - no R package exists to support development deep learning modeling of immune repertoire sequences. Several python packages have deep learning models that calculate based on Immune receptor sequences (deepTCR, deepCAT, Benisse). But this is still not the same thing. I have left the description of the functions arguments as part of the vignette.
positionalEncoder
man page appears in the in Details section rather than the Examples section. I think this is just a mistake with your roxygen2 markup%>%
is the only function imported into the NAMESPACE
from dplyr. To reduce the dependencies, either instead import it directly from magrittr (the package that actually defines it) or simply replace %>%
with |>
from basecovr::report()
for a helpful summary report of code coverage to identify areas that may be improved<-
rather than =
for assignment (e.g., es =
is used in vignette)BiocCheck::BiocCheck('immApex_0.99.0.tar.gz')
flags several things to fix (see abbreviated output below)
I have gone through and minimized the notes/warnings for the BiocCheck()
I will work on the build tomorrow and will send a quick update when I get it fixed.
Thanks, Nick
Received a valid push on git.bioconductor.org; starting a build for commit id: 5e264d80177b7a9fa206a879d1cef3bb6ff2432a
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/immApex
to trigger a new build.
A quick tutorial for setting up remotes and pushing to upstream can be found here.
Received a valid push on git.bioconductor.org; starting a build for commit id: 5e5719a02b4ee2de28cf193c5ca5eb06241fc2f5
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/immApex
to trigger a new build.
A quick tutorial for setting up remotes and pushing to upstream can be found here.
Received a valid push on git.bioconductor.org; starting a build for commit id: 4baed780ca31e7eb6b048e964da3d595bd63b1ed
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/immApex
to trigger a new build.
A quick tutorial for setting up remotes and pushing to upstream can be found here.
@PeteHaitch
It looks like python 3.12 is incompatible with keras/tensorflow installation. I tried to get around this by detecting if keras has been installed, and then installing python 3.10 and keras (per line 23 in vignette):
if(!keras::is_keras_available()) {
reticulate::install_python(version = '3.10')
suppressMessages(keras::install_keras(python_version = "3.10"))
}
This seems to still cause the same error, with the system resorting to python 3.12. Not sure what I can do from my end - maybe invoke a virtualenv, install everything, and then proceed with the vignette? This seems like a pretty tenuous solution. The problem is I don't seem to be able to replicate the issue on my system.
Let me know what you think or if you have any suggestions.
Nick
Hi @PeteHaitch
- [x] The vignette is not rendering properly in the built package, although it does when 'manually' rendered (e.g., via the knit button in RStudio). You can see the difference in the below screenshots. I've not seen this issue before, but perhaps @LiNk-NY can help understand what's happening?
I wasn't able to reproduce the rendering artifact. I installed the package with
R CMD build immApex
R CMD INSTALL immApex_0.99.4.tar.gz
and then opened the vignette in a browser via the command line with
browseVignettes(package = "immApex")
Best, Marcel
Hi @ncborcherding,
Thank you for addressing the initial review. I have some additional comments based on these changes and the discussion.
Cheers, Pete
It looks like python 3.12 is incompatible with keras/tensorflow installation. I tried to get around this by detecting if keras has been installed, and then installing python 3.10 and keras (per line 23 in vignette): This seems to still cause the same error, with the system resorting to python 3.12. Not sure what I can do from my end - maybe invoke a virtualenv, install everything, and then proceed with the vignette? This seems like a pretty tenuous solution. The problem is I don't seem to be able to replicate the issue on my system.
You must not install anything as part of the vignette.
I do not have a good understanding of the various versions of Python, tensorflow, and keras/keras3, that are available, nor how they well the interoperate. At times it feels like an ever-changing minefield...
However, I know that there has been considerable effort put into managing Python dependencies for Bioconductor packages. In particular, the basilisk package is designed to provide a consistent Python environment that can be used reliably by Bioconductor packages.
@vjcitn: Can you please advise @ncborcherding if basilisk would address their problem and how they might implement it?
In terms of other points:
Vignette should also include an 'Installation' section
I do not know how this would be done before bioconductor accepts the package? Should I add dummy instructions?
You can add these upon acceptance.
getIMGT()
.BiocCheck()
should be addressed (there are others, but I don't consider them essential for acceptance but they do reflect good programming practice).
> BiocCheck::BiocCheck('immApex_0.99.4.tar.gz')
<snip>
✔ Checking for 'LazyData: true' usage... [10ms] ℹ NOTE: 'LazyData:' in the 'DESCRIPTION' should be set to false or removed ✖ ERROR: Installation calls found in vignette(s) ! WARNING: Remove set.seed usage (found 1 times) • set.seed() in R/variationalSequences.R (line 139, column 3) ! WARNING: Empty or missing \format sections found in data man page(s). Found in files: • man/immapex_AA.data.Rd • ... • man/immapex_gene.list.Rd ℹ NOTE: Consider adding runnable examples to man pages that document exported objects. • getIR.Rd • variationalSequences.Rd ℹ NOTE: Usage of dontrun / donttest tags found in man page examples. 5% of man pages use at least one of these tags. Found in files: • variationalSequences.Rd ℹ NOTE: Use donttest instead of dontrun.
Received a valid push on git.bioconductor.org; starting a build for commit id: 1655fda0d8426a7f2dd805464b9c6abdf81dbe32
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/immApex
to trigger a new build.
A quick tutorial for setting up remotes and pushing to upstream can be found here.
@PeteHaitch
Thanks again, I got the minor formatting issue done in the vignette, updated the getIMGT()
per the guidelines you sent (https://contributions.bioconductor.org/querying-web-resources.html ), and went over the BiocCheck.
There are several remaining notes in BiocCheck, such as functions longer than 50 lines, lines > 80 characters, and multiples of 4 spaces.
It looks like the build is still erroring out due to the keras/python issue.
Thank you for making those changes, @ncborcherding, and for your constructive engagement throughout the review process.
I'm now happy to accept immApex into Bioconductor. Congratulations and thank you for your contribution!
Regarding the building/installation issues: since I've been able to successfully build and install it on my laptop, and we haven't heard back from @vjcitn, I'm happy to overlook this in favour of getting into the regular builds for the next Bioconductor release. Please keep a close eye on these builds and reach out on the bioc-devel mailing list or the Bioconductor Slack (particularly the #bioc-builds channel) if you need assistance in resolving any issues.
Your package has been accepted. It will be added to the Bioconductor nightly builds.
Thank you for contributing to Bioconductor!
Reviewers for Bioconductor packages are volunteers from the Bioconductor community. If you are interested in becoming a Bioconductor package reviewer, please see Reviewers Expectations.
@PeteHaitch Thanks for the helpful and thoughtful comments!
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/ncborcherding.keys is not empty), then no further steps are required. Otherwise, do the following:
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("immApex")
. The package 'landing page' will be created at
https://bioconductor.org/packages/immApex
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.
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]'
[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 instructions. My package is consistent with the Bioconductor Package Guidelines.
[x] I understand Bioconductor Package Naming Policy 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 genomic data.
[x] I am committed to the long-term maintenance of my package. This includes monitoring the support site for issues that users may have, subscribing to the 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 and agree to abide by it.
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.