Closed rcannood closed 6 years ago
Hello @rcannood and @zouter,
Thanks for considering CellTrails in your work. I am very sorry for my late response - I was out of office. We will have a look into your requests. What is your anticipated project timeline?
Best wishes, Daniel
Hi @dcellwanger
Thank you for your response!
Our current timeline is that we would generate the final benchmarking results from the 6th of august onwards, but hope to obtain the final versions of each method around the end of july.
Thanks! Wouter
Hi @zouter,
I browsed through your spreadsheet 'trajectory inference QC' and have the following comments on the CellTrails
sheet:
The method uses continuous integration, for example on Travis CI
We have added Travis CI integration. Please note that the most recent release (CellTrails
0.99.15) resolved of a minor bug which was arising due to the latest major update of one of its dependencies (ggplot2
).
The tutorial showcases the method on several datasets (1=0, 2=0.5, >2=1)
The paper shows the method's usefulness on several (1), one (0.25) or no real datasets.
The paper demonstrates that the CellTrails framework is suitable for a wide range of problems using
Package contains dummy proofing, i.e. testing whether the parameters and data supplied by the user make sense and are useful
To a certain degree, CellTrails
checks, indeed, wrong parameter settings which occurred during the pre-release package usage by the internal and external userbase; an exception is thrown or a default value is set as indicated by a warning message.
Cheers, Daniel
Hello Daniel,
Thanks for getting back in touch with us. I have a had a look at your comments and modified the QC scores accordingly.
Kind regards, Robrecht
QC23 (continuous integration): 0 β 1
If you like, you can add code coverage quite easily as well, by adding the following code to your .travis.yml
. This would increase the QC24 from 0 to 1 as well.
r_packages:
- covr
after_success:
- R CMD BiocCheck .
- R -e 'covr::codecov()'
QC34 (tutorial datasets): 0 β 1 QC49 (paper datasets): 0.25 β 1 I see! This was an oversight on our part.
QC20 (dummy proofing): 0 β 1 I had a closer look through the code and indeed saw dummy proofing for various exposed functions
Hello Robrecht,
Q24,25 Current code coverage = 81.63%
Q28,29 Version numbering and separation in devel/stable branches is somewhat tied to Bioconductor's life cycle (see Version Numbering), and therefore, meeting this criteria disadvantages novel Bioconductor packages.
Since its recent acceptance at Bioconductor, CellTrails
was only listed at Bioconductor's devel branch. I would argue that the code at GitHub represents the developmental version, while the code at Bioconductor is the stable version. Please note that CellTrails
had, indeed, several minor releases (see NEWS).
Biannually the Bioconductor devel branch becomes the Bioconductor release branch (in mid-April and mid-October). This will automatically generate a developmental branch of CellTrails
at Bioconductor (mirrored at GitHub), where new features are introduced, and a stable branch of CellTrails
at Bioconductor, respectively. Package version numbering is performed accordingly (see link above).
-Daniel
QC24: Nice! I updated it in the QC worksheet.
QC28,29: Fair enough. I will discuss this with @zouter. Perhaps all the bioconductor packages should get a score of 1 for this question. I
Hi Daniel,
QC28,29: We've changed the values of these scores to '1' for all bioconductor packages. This results in CellTrails having a near-perfect QC score.
I'm closing this issue for now. If you have any further comments on the CellTrails wrapper, feel free to create a new issue or pull request.
Robrecht
Hello @dcellwanger
This issue is for discussing the wrapper for your trajectory inference method, CellTrails, which we wrapped for our benchmarking study (10.1101/276907). In our dynmethods framework, we collected some meta information about your method, and created a docker wrapper so that all methods can be easily run and compared. The code for this wrapper is located in a docker container. The way this container is structured is described in this vignette.
We are creating this issue to ensure your method is being evaluated in the way it was designed for. The checklist below contains some important questions for you to have a look at.
definition.yml
, and is expected to produce certain output files, also as defined bydefinition.yml
.The most convenient way for you to test and adapt the wrapper is to install dyno, download and modify these files, and run your method on a dataset of interest or one of our synthetic toy datasets. This is further described in this vignette. Once finished, we prefer that you fork the dynmethods repository, make the necessary changes, and send us a pull request. Alternatively, you can also send us the files and we will make the necessary changes.
If you have any further questions or remarks, feel free to reply to this issue.
Kind regards, @zouter and @rcannood