New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
citesdb: A high-performance database of shipment-level CITES trade data #292
Comments
Thank you for submitting, @noamross! I will be your editor, so please hold on while I perform editor checks. |
Editor checks:
Editor commentsThanks for your submission @noamross! As the output from
Reviewers: @pachamaltese @xavi-rp |
Hello @noamross! @pachamaltese and @xavi-rp have been assigned as reviewers. They will have until May 6th to review the package. |
Hi @noamross, @melvidoni and @xavi-rp
|
Hi @pachamaltese, that should be taken care of now. |
Package Review
DocumentationThe package includes all the following forms of documentation:
Functionality
Final approval (post-review)
Estimated hours spent reviewing: 4 hours Review CommentsI enjoyed reviewing the Below I've highlighted some observations that might make this package even better. DocumentationPackage Metadata
Package Documentation
Vignettes
Function Documentation
Functionality / ImplementationExternal Dependencies Section 1.11 of rOpenSci's packaging guides reinforces the good practice of trying to limit Readability / Modularity
Technical Implementation Details
Testing
Error Messages
General commentsLike I stated above, there are some fixes that are needed and may improve the package a lot. The code is easy to read, without strange lines, and my overall impression of it is positive. 65% of test coverage seems to be very low. By adding some elementary checks, this number can be increased a lot. Unlike my 1st submission to rOpenSci, most of the functions in citedb have adecuate names and can be easily distinguished as those names are not generic (i.e. "filter" or "download"). |
Note to reviewers: The MonetDBLite package has been (hopefully temporarily) archived on CRAN, so I have switched the citesdb to using a stable GitHub version of MonetDBLite via the |
Hi @noamross and @melvidoni, Package Review
DocumentationThe package includes all the following forms of documentation:
Functionality
Final approval (post-review)
Estimated hours spent reviewing: 8 (a lot of time spent to try to understand the problem with the vignettes) Review CommentsI find this package easy to install/use and very useful to explore and use the data from the CITES Trade DB, so congrats to the authors! It's very useful the inclusion of the metadata functions. And also the gif!!! The main problem that I found is in the vignette building while running In
The spelling errors found after running Other minor comments:
Congrats again to the authors!! |
Thank you for your careful reviews, @pachamaltese and @xavi-rp! We'll get to work on these. |
Response to reviewersThanks to both @pachamaltese and @xavi-rp for their comments and issues that have Changes implemented
Changes not implemented / justifications
A note: we have learned from the maintainers of MonetDBLite that it will |
Hi, |
Thank you @xavi-rp, noted. @pachamaltese what are your thoughts? |
@melvidoni I totally agree with this:
|
Approved! Thanks @noamross for submitting and @pachamaltese and @xavi-rp for your reviews! To-dos:
Should you want to acknowledge your reviewers in your package DESCRIPTION, you can do so by making them Welcome aboard! We'd love to host a blog post about your package - either a short introduction to it with one example or a longer post with some narrative about its development or something you learned, and an example of its use. If you are interested, review the instructions, and tag @stefaniebutland in your reply. She will get in touch about timing and can answer any questions. We've started putting together a gitbook with our best practice and tips, this chapter starts the 3d section that's about guidance for after onboarding. Please tell us what could be improved, the corresponding repo is here. |
@melvidoni IMO there's an additional pending: to apply rOpenSci themes to their vignettes |
@pachamaltese You just told me before that it was ready to be approved... |
Thanks all very much, I believe the RO theme will be applied automatically upon repository transfer and building on @pachamaltese and @xavi-rp, we'd like to acknowledge you as |
Yes, that's my orcid! Thanks!! |
Correct id!
You have to download the ropensci theme and edit _pkgdown.yml to create an
ropensci-themed pkgdown site
Check github.com/ropensci/tradestatistics for the details
…On Wed, May 29, 2019, 9:45 AM Noam Ross ***@***.***> wrote:
Thanks all very much, I believe the RO theme will be applied automatically
upon repository transfer and building on docs.ropensci.org.
@pachamaltese <https://github.com/pachamaltese> and @xavi-rp
<https://github.com/xavi-rp>, we'd like to acknowledge you as rev authors
in our DESCRIPTION if that's all right. I believe your ORCiDs are
https://orcid.org/0000-0003-2046-0621 (Xavier) and 0000-0003-1017-7574
(Pachá), correct?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#292?email_source=notifications&email_token=ACM7UOOFOQSEYV75YX25KMTPX2CFVA5CNFSM4HD65KAKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODWPL6GI#issuecomment-496942873>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ACM7UOMWQ7EEWY4XI5MB2STPX2CFVANCNFSM4HD65KAA>
.
|
I've checked off everything except the footer, which I'm leaving off in anticipation of a change in workflow: ropensci/software-review-meta#79, and will be in touch with Stefanie about a blog post. Thanks all! |
@noamross I saw ropensci.github.io/citesdb
It just needs the rOpenSci theme and that's it! Just 2 minutes away from
the best result
…On Wed, May 29, 2019, 1:03 PM Noam Ross ***@***.***> wrote:
I've checked off everything except the footer, which I'm leaving off in
anticipation of a change in workflow: ropensci/software-review-meta#79
<ropensci/software-review-meta#79>, and will be
in touch with Stefanie about a blog post. Thanks all!
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#292?email_source=notifications&email_token=ACM7UOIIUFUDQ65VMSMLMATPX2ZOHA5CNFSM4HD65KAKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODWP7YDY#issuecomment-497024015>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ACM7UOOSONYFUFXBC47LWT3PX2ZOHANCNFSM4HD65KAA>
.
|
@pachamaltese we are about to change the guidance actually because @jeroen has been working on centrally building all docs websites so authors do not need to worry about this. The pkgdown config is used but not for styling since rotemplate is used for all. See e.g. docs.ropensci.org/magick. |
Thanks @maelle!! It was different when I submitted
…On Wed, May 29, 2019, 2:15 PM Maëlle Salmon ***@***.***> wrote:
@pachamaltese <https://github.com/pachamaltese> we are about to change
the guidance actually because @jeroen <https://github.com/jeroen> has
been working on centrally building all docs websites so authors do not need
to worry about this. The pkgdown config is used but not for styling since
rotemplate is used for all. See e.g. docs.ropensci.org/magick.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#292?email_source=notifications&email_token=ACM7UOM6RIYDJMV4IU3PEH3PX3B2XA5CNFSM4HD65KAKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODWQGHAA#issuecomment-497050496>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ACM7UON4LFIHCSZL6Z4RM5DPX3B2XANCNFSM4HD65KAA>
.
|
Indeed, very recent change! |
Submitting Author: Noam Ross (@noamross)
Repository: https://github.com/ecohealthalliance/citesdb
Version submitted: 0.1.0
Editor: @melvidoni
Reviewer 1: @xavi-rp
Reviewer 2: @pachamaltese
Archive: https://zenodo.org/record/3234871
Version accepted: 0.2.0
Scope
Please indicate which category or categories from our package fit policies this package falls under: (Please check an appropriate box below. If you are unsure, we suggest you make a pre-submission inquiry.):
Explain how and why the package falls under these categories (briefly, 1-2 sentences):
citesdb gives researchers access to the large CITES wildlife trade dataset by syncing it to a local out-of-memory trade database and provides an interface to that local database.
Researchers and conservationists studying the wildlife trade.
Our deprecated cites package used a different framework to access a smaller subset of the data. The rcites package provides complementary data.
Technical checks
Confirm each of the following by checking the box. This package:
I note the relatively low test coverage (~65%) is due to a verbose section of code configuring the RStudio interactive interface. Other than this, the remaining code has >90% coverage.
Publication options
JOSS Options
paper.md
matching JOSS's requirements with a high-level description in the package root or ininst/
.MEE Options
Code of conduct
Recommended reviewers: karinorman will be familiar with the database used here via her work on taxadb, JonasGeschke and KevCaz will know about the data and their application through their work on rcites. Feedback from someone involved in metadata projects - e.g.: Bryce Mecum, Auriel Fournier, Kelly Hondula - would also be valuable.
The text was updated successfully, but these errors were encountered: