search

Bioconductor / Contributions

Contribute Packages to Bioconductor
135 stars 33 forks source link

microSTASIS #2795

Closed BiotechPedro closed 2 years ago

BiotechPedro commented 2 years ago

"The way this is usually done is that you get your package added to BioC devel and only this gets released we archive the package from CRAN".

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 2 years ago

Hi @BiotechPedro

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: microSTASIS
Title: Microbiota STability ASsessment via Iterative cluStering
Version: 0.99.0
Description: The toolkit 'µSTASIS' has been developed for the stability analysis of microbiota in a temporal framework by leveraging on iterative clustering. Concretely, the core function uses Hartigan-Wong k-means algorithm as many times as possible for stressing out paired samples from the same individuals to test if they remain together for multiple numbers of clusters over a whole data set of individuals. Moreover, the package includes multiple functions to subset samples from paired times, validate the results or visualize the output.
Authors@R: c(
    person(given = "Pedro",
 family = "Sánchez-Sánchez",
 role = c("aut", "cre"),
 email = "bio.pedro.technology@gmail.com",
 comment = c(ORCID = "0000-0002-4846-1813")),
    person(given = "Alfonso",
 family = "Benítez-Páez",
 role = c("aut"),
 email = "alfbenpa@gmail.com",
 comment = c(ORCID = "0000-0001-5707-4340")))
License: GPL-3
Imports: 
    BiocParallel,
    ggplot2,
    ggside,
    grid,
    rlang,
    stats,
    stringr
Suggests:
    BiocStyle,
    gghighlight,
    knitr,
    rmarkdown,
    methods,
    RefManageR,
    sessioninfo,
    SingleCellExperiment,
    SummarizedExperiment,
    TreeSummarizedExperiment,
    testthat (>= 3.0.0)
biocViews: GeneticVariability, BiomedicalInformatics, Clustering, MultipleComparison, Microbiome
BugReports: https://github.com/BiotechPedro/microSTASIS
URL: https://doi.org/10.1093/bib/bbac055
Encoding: UTF-8
LazyData: true
Roxygen: list(markdown = TRUE)
RoxygenNote: 7.2.1
VignetteBuilder: knitr
Depends: 
    R (>= 4.2.0)
Config/testthat/edition: 3
bioc-issue-bot commented 2 years ago

A reviewer has been assigned to your package. Learn what to expect during the review process.

IMPORTANT: Please read this documentation for setting up remotes to push to git.bioconductor.org. It is required to push a version bump to git.bioconductor.org to trigger a new build.

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 2 years ago

Dear Package contributor,

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

Your package has been built on Linux, Mac, and Windows.

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

lazappi commented 2 years ago

Pinging @lshep who wanted to keep an eye on this issue

lazappi commented 2 years ago

Hi @BiotechPedro

Thanks for submitting microSTASIS :tada:! 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: :rotating_light: Required :warning: Recommended :green_circle: Optional :question: Question

General package development

DESCRIPTION file

The NEWS file

The CITATION file

Documentation

Vignette

Man pages

Code

R

BiotechPedro commented 2 years ago

Thanks Luke!!

I will try to reply with changes ASAP, although I have an important deadline next Tuesday. I will give my utmost to have it everything answer by the next Friday/Saturday.

Cheers,

Pedro

BiotechPedro commented 2 years ago

Hi @lazappi ! Here it's my reply. The major issues is the one of LazyData.

Review

Key: 🚨 Required ⚠️ Recommended 🟢 Optional ❓ Question

General package development

  • [ ] 🚨 Just making a note about the CRAN issue here
  • [x] ⚠️ There are some BiocCheck::BiocCheck() NOTES in the build report, please address as many of these as possible

I know there were 3 notes. I'll try to address them by following your following comments, although with the few time I have is not my priority right now.

DESCRIPTION file

  • [x] 🟢 It is possible to split the Description field over multiple lines which would make it easier to read

Done!

  • [!!!] 🚨 Bioconductor recommends setting LazyData: FALSE

🚨⚠️ When removing the line of LazyData or setting it as false, there is a problem with loading the data at the vignette. That's why I left it that way.

  • [x] 🟢 The BugReports field usually links directly to the issues page of your GitHub repository (or the Bioconductor support forum)

❓ Should I remove that line then or leave it with the GitHub repo?

  • [x] 🟢 The URL field is usually a link to your GitHub repository or the Bioconductor package page (or both), I guess a link to the paper is a reasonable alternative/addition though

I prefer to set the link to the paper.

The NEWS file

  • [x] 🚨 Check the format of the NEWS file, when I installed the package and tried utils::news(package = "microSTASIS") it didn't render properly

When I do devtools::load_all and then utils::news(package = "microSTASIS"), it prints it in the Help window 🫤

The CITATION file

  • [x] 🚨 I got an error when I tried to read the CITATION file with readCitationFile("inst/CITATION")

Totally true. I removed the accent in our surnames. I think is the best option.

Documentation

  • [x] ⚠️ I think it's a bit confusing that the name of the package is "microSTASIS" but you sometimes refer to it as "µSTASIS"

Yes! I understand it, but my former PI and I decided to write it that way because it is a package build in relation to microbiome data and, therefore, Microbiology. I believe it doesn't add too much confusion to the users.

Vignette

  • [x] 🚨 Please include some text in the introduction explaining your motivation for including the package in Bioconductor (compatibility with object/workflows etc.)

Done! It's a fair point.

  • [x] 🚨 If there are any existing packages that performa a similar function please mention them in the introduction

Our metric is very unique since we didn't find other packages leveraging on iterative clustering for assessing the stability of high-dimensional microbiome data. Also, there are few metrics in the CoDA framework, but we don't feel they perform similar functions.

  • [x] ⚠️ The vignette does a good job of showing the workflow of the package but there is not a lot of description of what is happening at each stage. A good rule-of-thumb is that every code chunk should have some text before that says what you are about to do and some text after that explains the output.

Totally agree. Improved!

  • [x] ⚠️ Please show the input data and explain what format it should have

Done!

  • [x] ⚠️ You should also add some text describing the plots that are produced and how to interpret them

Done!

  • [X] ⚠️ If there are an particularly important parameters or things that need to be manually set by the user please desribe these as well

Oh right! Done.

  • [x] 🚨 Please add a heading to the sessionInfo() section

Done.

  • [x] 🚨 When I rendered the vignette the image in the Introduction didn't display correctly, please check this

My fault. Now it's corrected.

  • [x] ❓ What do the [comment]: <> () sections do? I haven't seen this before.

It is for commenting markdown code and not to print it. It's like # in the R code chunks.

  • [x] 🚨 Some of these are not rendered properly (you can see the code in the output)

I have removed them.

  • [x] 🚨 If you want to hide messages when loading packages you should use suppressPackageStartupMessages(library(PACKAGE)) rather than setting message=FALSE

Didn't know. Thanks.

Man pages

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

Done!

Code

R

  • [x] 🚨 I think there are a few places where you use @ or slot() to access data rather than an accessor function (pairedTimes.R L47)

I missed that line. Now it's changed. I have checked it and haven't seen anything more.

  • [x] 🚨 In your function arguments documentation you should describe the format of the input and/or the function which can be used to produce it

I think this was already done. I took care of it since it is something very important for me as a user. Still, I have rewritten some of them to be clearer.

  • [x] ⚠️ I found the name of the common argument to be a bit confusing, I think this would usually be named something like sep or pattern

I think is good enough. Easily to understand by looking at the help of the functions and explained in the vignette.

  • [x] 🚨 Instead of having a logical parallel argument please use a BPPARAM argument that lets the user supply a BiocParallel parameters object

It's a better option. Done!

  • [x] 🚨 Please clean up L27 in iterativeClustering(), currently it's not doing anything because it's immediately replaced by the next line. It might be worth checking if there are any other cases like this that I missed.

It should be the only case. It's a leftover from the former code.

  • [x] ⚠️ I found the description of the k parameter in your cross-validation functions a bit confusing. Usually k-fold CV means splitting the data into k subsets not removing k observations at a time. Maybe I misunderstood the documentation but I think this could be confusing for users.

I was taught the other way! 😐 What can be the better way to explain it? I have written in the description of iterativeClusteringCV: "Perform cross validation of the stability results from [microSTASIS::iterativeClustering()]in the way of leave-one-out (LOO) or k-fold (understood as quitting k individuals each time for assessing calculating the metric over individuals/k subsets of the data).". Also, the explanation of the argument k seems clear to me.

  • [x] ⚠️ When you are using the CV results in another function is there a way you can get k automatically rather than need to the user to supply it?

Good point. I wish but don't think there is a clear way. I have put a recommendation in the vignette.

  • [x] 🚨 When you need the number of rows or columns please use nrow()/ncol() rather than dim()[1]/dim()[2]

Ok. Done!

  • [x] 🚨 When writing if statements please negate the condition rather than having an empty if block. So:

Yes. That lines from pairedTimes() are now correct.

  • [x] ⚠️ I think some of you function names could be clearer, especially those for plotting (it's not clear that mSdynamics() makes a boxplot for example)

Yes, I agree it is not super clear. But I prefer to call it that way because of understanding the real aim of that visualization.

  • [x] ⚠️ In mSmetadataGroups it is unclear to me why you need special arguments for TreeSummarizedExperiment objects. If this is necessary it's better to test the input in the function using is(metadata, "TreeSummarizedExperiment") than require the user to specify

I put it that way because if using TreeSummarizedExperiment, the data will be normally split in columns and not in a single character vector of "individual_common_timepoint". For that I need the if statement. Also, I cannot use the is(metadata, "TreeSummarizedExperiment") because metadata is a data.frame, coming from colData(). Other option could be to put that metadata = TSEobject and use that is(...) but I don't like that because the information can be as colData() of one assay of TSE or also as colData() from altExp() slot. That's why I wrote it that way.

  • [x] ⚠️ The convention is that mandatory function arguments (those without a default value) are specified first and then optional arguments. This makes it easier for the user to see which things they must set and which are optional.

Ok. Didn't know. Done!

  • [x] 🟢 I found your code overall a bit messy and hard to follow. Here are some suggestions how I would tidy it up but up to you which you decide to use.
    • [] 🟢 When you repeatedly need to extract some information from an object it is clearer (and more efficient) to do it once and reuse the variable.
    • [] 🟢 Try to limit how much code is inside if statements
    • [] 🟢 Adding some comments or even just whitespace between sections of functions would make things a bit easier to read
    • [] 🟢 You might want to try stying the R files with something like styler::style_pkg(transformers = biocthis::bioc_style()). Even if you don't like how it formats things it might give you some ideas. Make sure you have a commit you can go back to before you try this though!

I absolutely agree in all the suggestions. Will be better in the future.

Thanks! Looking forward to your comments.

Pedro

lazappi commented 2 years ago

@BiotechPedro You need to push the changes you have made to the Bioconductor git server so I can review them. I will reply to some of your comments below but I need to see the code for others.

  • [ ] 🚨 Just making a note about the CRAN issue here

  • [!!!] 🚨 Bioconductor recommends setting LazyData: FALSE

🚨⚠️ When removing the line of LazyData or setting it as false, there is a problem with loading the data at the vignette. That's why I left it that way.

Hmmm...ok. Do you know what the issue was? This shouldn't really affect whether you can load the data or not.

  • [x] 🟢 The BugReports field usually links directly to the issues page of your GitHub repository (or the Bioconductor support forum)

❓ Should I remove that line then or leave it with the GitHub repo?

Up to you, I just wanted to point out what is usually done here.

  • [x] 🟢 The URL field is usually a link to your GitHub repository or the Bioconductor package page (or both), I guess a link to the paper is a reasonable alternative/addition though

I prefer to set the link to the paper.

Fair enough. You can also set multiple URLs if you want to have both.

  • [x] 🚨 Check the format of the NEWS file, when I installed the package and tried utils::news(package = "microSTASIS") it didn't render properly

When I do devtools::load_all and then utils::news(package = "microSTASIS"), it prints it in the Help window 🫤

It mostly looks ok but the second level of dot points isn't rendered so you just get a chunk of text for each point instead of a sublist.

image

  • [x] 🚨 I got an error when I tried to read the CITATION file with readCitationFile("inst/CITATION")

Totally true. I removed the accent in our surnames. I think is the best option.

I need the updated code to test this but that should work. It's a bit disappointing you can't include accents though, I had a quick look but I couldn't find a better solution.

  • [x] ⚠️ I think it's a bit confusing that the name of the package is "microSTASIS" but you sometimes refer to it as "µSTASIS"

Yes! I understand it, but my former PI and I decided to write it that way because it is a package build in relation to microbiome data and, therefore, Microbiology. I believe it doesn't add too much confusion to the users.

Could you add a sentence to the vignette (and maybe the README) explaining this for users?

  • [x] 🚨 If there are any existing packages that performa a similar function please mention them in the introduction

Our metric is very unique since we didn't find other packages leveraging on iterative clustering for assessing the stability of high-dimensional microbiome data. Also, there are few metrics in the CoDA framework, but we don't feel they perform similar functions.

Are there other packages for clustering microbiome data even if it is in a different way? It's also ok to point out the differences with your package, something like "Package X and package Y do Task A but we do Task B which is different because of Z".

  • [x] ❓ What do the [comment]: <> () sections do? I haven't seen this before.

It is for commenting markdown code and not to print it. It's like # in the R code chunks.

Ok, cool. I haven't seen that used before, just the HTML style <!---...---> comments

  • [x] ⚠️ I found the name of the common argument to be a bit confusing, I think this would usually be named something like sep or pattern

I think is good enough. Easily to understand by looking at the help of the functions and explained in the vignette.

I actually found in quite unclear which is why I mentioned it. If you have made changes to the vignette that explain it better it's probably ok though.

  • [x] ⚠️ I found the description of the k parameter in your cross-validation functions a bit confusing. Usually k-fold CV means splitting the data into k subsets not removing k observations at a time. Maybe I misunderstood the documentation but I think this could be confusing for users.

I was taught the other way! 😐 What can be the better way to explain it? I have written in the description of iterativeClusteringCV: "Perform cross validation of the stability results from [microSTASIS::iterativeClustering()]in the way of leave-one-out (LOO) or k-fold (understood as quitting k individuals each time for assessing calculating the metric over individuals/k subsets of the data).". Also, the explanation of the argument k seems clear to me.

Your explanation is ok but what you are describing is Leave p (or k) Out Cross Validation, not k-fold Cross Validation. There is a good quick description of common CV methods here. Using either is fine but your description should match the method you are using.

  • [x] ⚠️ I think some of you function names could be clearer, especially those for plotting (it's not clear that mSdynamics() makes a boxplot for example)

Yes, I agree it is not super clear. But I prefer to call it that way because of understanding the real aim of that visualization.

Could you at least call it something like plot_mSdynamics() to make it clear this is a plotting function not an analysis function?

  • [x] ⚠️ In mSmetadataGroups it is unclear to me why you need special arguments for TreeSummarizedExperiment objects. If this is necessary it's better to test the input in the function using is(metadata, "TreeSummarizedExperiment") than require the user to specify

I put it that way because if using TreeSummarizedExperiment, the data will be normally split in columns and not in a single character vector of "individual_common_timepoint". For that I need the if statement. Also, I cannot use the is(metadata, "TreeSummarizedExperiment") because metadata is a data.frame, coming from colData(). Other option could be to put that metadata = TSEobject and use that is(...) but I don't like that because the information can be as colData() of one assay of TSE or also as colData() from altExp() slot. That's why I wrote it that way.

I think this will be quite confusing to users the way it is, I'm struggling to understand it even after looking at the code. I think the issue is that you are extracting the same information from different data structures in different ways and it's not clear what the inputs can be or which arguments apply to different inputs. I think you can probably rewrite it to reuse some of the arguments, or maybe it would just be easier to have two functions.

BiotechPedro commented 2 years ago

Hello @lazappi :D to make it short I only quote questions or comments that have been answer.

@BiotechPedro You need to push the changes you have made to the Bioconductor git server so I can review them. I will reply to some of your comments below but I need to see the code for others.

✅ Yes. Now it's done.

  • [x] 🚨 Just making a note about the CRAN issue here

✅ I have written to CRAN maintaining for already archiving the package so the package has no errors for the 28th.

  • [x] 🚨 Bioconductor recommends setting LazyData: FALSE

🚨⚠️ When removing the line of LazyData or setting it as false, there is a problem with loading the data at the vignette. That's why I left it that way.

Hmmm...ok. Do you know what the issue was? This shouldn't really affect whether you can load the data or not.

✅ Now it should be ok. I set LazyData: FALSE and put data(clr). I set @usage data(clr) in the data.R file. It seems ok after check() and BiocCheck().

  • [x] 🚨 Check the format of the NEWS file, when I installed the package and tried utils::news(package = "microSTASIS") it didn't render properly

When I do devtools::load_all and then utils::news(package = "microSTASIS"), it prints it in the Help window 🫤

It mostly looks ok but the second level of dot points isn't rendered so you just get a chunk of text for each point instead of a sublist.

image

✅ Thanks. Now the file has been modified.

  • [x] ⚠️ I think it's a bit confusing that the name of the package is "microSTASIS" but you sometimes refer to it as "µSTASIS"

Yes! I understand it, but my former PI and I decided to write it that way because it is a package build in relation to microbiome data and, therefore, Microbiology. I believe it doesn't add too much confusion to the users.

Could you add a sentence to the vignette (and maybe the README) explaining this for users?

✅ Yes, good point. It's done.

  • [x] 🚨 If there are any existing packages that performa a similar function please mention them in the introduction

Our metric is very unique since we didn't find other packages leveraging on iterative clustering for assessing the stability of high-dimensional microbiome data. Also, there are few metrics in the CoDA framework, but we don't feel they perform similar functions.

Are there other packages for clustering microbiome data even if it is in a different way? It's also ok to point out the differences with your package, something like "Package X and package Y do Task A but we do Task B which is different because of Z".

❗️ I prefer not to include this because there are no similar packages in that sense. Rather than clustering microbiome data, we use clustering in an iterative way for assessing the stability, which can also be understood as a metric. Therefore, we don't find there are others packages for assessing stability in microbiota across in a longitudinal context.

  • [x] ⚠️ I found the description of the k parameter in your cross-validation functions a bit confusing. Usually k-fold CV means splitting the data into k subsets not removing k observations at a time. Maybe I misunderstood the documentation but I think this could be confusing for users.

I was taught the other way! 😐 What can be the better way to explain it? I have written in the description of iterativeClusteringCV: "Perform cross validation of the stability results from [microSTASIS::iterativeClustering()]in the way of leave-one-out (LOO) or k-fold (understood as quitting k individuals each time for assessing calculating the metric over individuals/k subsets of the data).". Also, the explanation of the argument k seems clear to me.

Your explanation is ok but what you are describing is Leave p (or k) Out Cross Validation, not k-fold Cross Validation. There is a good quick description of common CV methods here. Using either is fine but your description should match the method you are using.

✅ You're right. It's the proper way to explain it. I have change every explanation regarding k-fold for leave-p-out (or k) CV.

  • [x] ⚠️ I think some of you function names could be clearer, especially those for plotting (it's not clear that mSdynamics() makes a boxplot for example)

Yes, I agree it is not super clear. But I prefer to call it that way because of understanding the real aim of that visualization.

Could you at least call it something like plot_mSdynamics() to make it clear this is a plotting function not an analysis function?

✅ I have thought again about this and I also like to clearly now the functions for plotting rather than analysis. Now there are four functions with a new name. Also, I avoided underscores and wrote the names as camelCase as indicated here

  • [x] ⚠️ In mSmetadataGroups it is unclear to me why you need special arguments for TreeSummarizedExperiment objects. If this is necessary it's better to test the input in the function using is(metadata, "TreeSummarizedExperiment") than require the user to specify

I put it that way because if using TreeSummarizedExperiment, the data will be normally split in columns and not in a single character vector of "individual_common_timepoint". For that I need the if statement. Also, I cannot use the is(metadata, "TreeSummarizedExperiment") because metadata is a data.frame, coming from colData(). Other option could be to put that metadata = TSEobject and use that is(...) but I don't like that because the information can be as colData() of one assay of TSE or also as colData() from altExp() slot. That's why I wrote it that way.

I think this will be quite confusing to users the way it is, I'm struggling to understand it even after looking at the code. I think the issue is that you are extracting the same information from different data structures in different ways and it's not clear what the inputs can be or which arguments apply to different inputs. I think you can probably rewrite it to reuse some of the arguments, or maybe it would just be easier to have two functions.

✅ Totally agree. I have rewritten the help of the function and the function itself.

Best,

Pedro

lazappi commented 2 years ago

@BiotechPedro I haven't read your replies yet but I don't see any updates from the build system. Were you able to get pushing to the Bioconductor git server to work?

BiotechPedro commented 2 years ago

I computed git remote add upstream git@git.bioconductor.org:packages/microSTASIS.git git push upstream main:master and the printed output was:

Enumerating objects: 134, done. Counting objects: 100% (134/134), done. Delta compression using up to 8 threads Compressing objects: 100% (87/87), done. Writing objects: 100% (90/90), 20.63 KiB | 186.00 KiB/s, done. Total 90 (delta 66), reused 0 (delta 0), pack-reused 0 To git.bioconductor.org:packages/microSTASIS.git 0345fe3..8a7fe3c main -> master

I will check it everything right now and will come later.

Sorry for the inconvenience :/

BiotechPedro commented 2 years ago

Hi @lazappi I have recheck everything. I have done git clone git@git.bioconductor.org:packages/microSTASIS.git and I download the correct repo. For example, the last commit I did includes changing the plotting function names. When the repo is cloned, the names are changed as should be.

lazappi commented 2 years ago

Hmmm...ok. When I pull from Bioconductor I can see your changes as well so git has worked but for some reason there's no build. @Bioconductor/core any ideas what might be happening here?

lshep commented 2 years ago

The changes won't register or trigger a new build without a version bump. Please bump the version to 0.99.1 and push and it should trigger a build

bioc-issue-bot commented 2 years ago

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

BiotechPedro commented 2 years ago

Thanks for the help @lazappi @lshep I feel super bad about being so naive. I didn't change the version which is obviously needed for the build. Sorry. Now it is fixed.

bioc-issue-bot commented 2 years ago

Dear Package contributor,

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

Your package has been built on Linux, Mac, and Windows.

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

lazappi commented 2 years ago

No worries about the version bumping thing. It happens all the time and I should have thought of that straight away.

I'll reply to the conversation about the non-code points first:

  • [x] 🚨 If there are any existing packages that performa a similar function please mention them in the introduction

Our metric is very unique since we didn't find other packages leveraging on iterative clustering for assessing the stability of high-dimensional microbiome data. Also, there are few metrics in the CoDA framework, but we don't feel they perform similar functions.

Are there other packages for clustering microbiome data even if it is in a different way? It's also ok to point out the differences with your package, something like "Package X and package Y do Task A but we do Task B which is different because of Z".

❗️ I prefer not to include this because there are no similar packages in that sense. Rather than clustering microbiome data, we use clustering in an iterative way for assessing the stability, which can also be understood as a metric. Therefore, we don't find there are others packages for assessing stability in microbiota across in a longitudinal context.

Ok, if you don't think there is anything relevant to mention that's fine. Maybe something to keep in mind for future updates to the vignette though.

I think the code changes are good so no further comments on those.

Only thing I noticed is when you load the package you get the following message:

in method for ‘pairedTimes’ with signature ‘"TreeSummarizedExperiment"’: no definition for class “TreeSummarizedExperiment”

I think you can fix this by moving {TreeSummarizedExperiment} from Suggests: to Imports:

lazappi commented 2 years ago

@Bioconductor/core I'm happy with this from the review side but I haven't added the accepted tag because it is currently still on CRAN.

bioc-issue-bot commented 2 years ago

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

BiotechPedro commented 2 years ago

Hi @lazappi ! Thanks for the imports suggestion. Done! I will contact again to CRAN for archiving it. They told me it should be on Bioc-devel. I will attached them your comment and will return ASAP.

Thanks,

Pedro

bioc-issue-bot commented 2 years ago

Dear Package contributor,

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

Your package has been built on Linux, Mac, and Windows.

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

BiotechPedro commented 2 years ago

Hello @lazappi @lshep

I have put you in copy in a mail conversation with CRAN team for discussing about the way to move the package to Bioconductor and archiving it on CRAN.

Thank you!

lshep commented 2 years ago

@lazappi per the email conversation we will ingest first and then remove from CRAN. If the package is ready otherwise please update labels appropriately.

lshep commented 2 years ago

The master branch of your GitHub repository has been added to Bioconductor's git repository.

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/BiotechPedro.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("microSTASIS"). The package 'landing page' will be created at

https://bioconductor.org/packages/microSTASIS

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.