Skip to content
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

submission: dbparser #347

Closed
MohammedFCIS opened this issue Oct 24, 2019 · 31 comments
Closed

submission: dbparser #347

MohammedFCIS opened this issue Oct 24, 2019 · 31 comments

Comments

@MohammedFCIS
Copy link

@MohammedFCIS MohammedFCIS commented Oct 24, 2019

Submitting Author: Mohammed Ali(@submission:)
Repository: https://github.com/Dainanahan/dbparser
Version submitted: v1.0.4
Editor: @noamross
Reviewer 1: @emmamendelsohn
Reviewer 2: @haozhu233
Archive: TBD
Version accepted: TBD


  • Paste the full DESCRIPTION file inside a code block below:
Package: dbparser
Title: 'DrugBank' Database XML Parser
Version: 1.0.4
Authors@R:
    c(
    person("Mohammed", "Ali", email = "moh_fcis@yahoo.com", role = c("aut", "cre")),
    person("Ali", "Ezzat", email = "ali_ezzat85@yahoo.com", role = "aut"))
Description: This tool is for parsing the 'DrugBank' XML database <http://drugbank.ca/>. The parsed 
    data are then returned in a proper 'R' dataframe with the ability to save 
    them in a given database.
License: MIT + file LICENSE
Encoding: UTF-8
LazyData: true
Imports:
    DBI,
    odbc,
    purrr,
    readr,
    RMariaDB,
    tibble,
    tidyr,
    tools,
    XML
RoxygenNote: 6.1.1
Suggests:
    knitr,
    rmarkdown,
    testthat
VignetteBuilder: knitr
URL: https://dainanahan.github.io/dbparser/index.html
BugReports: https://github.com/Dainanahan/dbparser/issues

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.):

    • data retrieval
    • data extraction
    • database access
    • data munging
    • data deposition
    • workflow automataion
    • version control
    • citation management and bibliometrics
    • scientific software wrappers
    • database software bindings
    • geospatial data
    • text analysis
  • Explain how and why the package falls under these categories (briefly, 1-2 sentences):
    The package accesses DrugBank DB, extract and transform data into R tibbles, and provides a set of functions to save them in a given SQL database.

  • Who is the target audience and what are scientific applications of this package?
    Pharmaceutical researches who are interested to use data science and machine learning tools in their Drugs Development research

  • Are there other R packages that accomplish the same thing? If so, how does yours differ or meet
    There is drugbankR that might do a similar task by loading the whole DrugBank database into a single R dataframe and provide some queries to for specific data.
    On the other hand, dbparser extract drugbank database in to different 75 scientifically detailed dataframe with clear attributes. These dataframes need no extra functionalities to query them except regular base R functionalities. Also these dataframe can be saved in to a given SQL database.
    Also, dbparser comes to greater coverage in terms of functionality, documentation, and robust test coverage and error handling, In comparison to drugbankR.

  • If you made a pre-submission enquiry, please paste the link to the corresponding issue, forum post, or other discussion, or @tag the editor you contacted.

Technical checks

Confirm each of the following by checking the box. This package:

Publication options

JOSS Options
  • The package has an obvious research application according to JOSS's definition.
    • The package contains a paper.md matching JOSS's requirements with a high-level description in the package root or in inst/.
    • The package is deposited in a long-term repository with the DOI:
    • (Do not submit your package separately to JOSS)
MEE Options
  • The package is novel and will be of interest to the broad readership of the journal.
  • The manuscript describing the package is no longer than 3000 words.
  • You intend to archive the code for the package in a long-term repository which meets the requirements of the journal (see MEE's Policy on Publishing Code)
  • (Scope: Do consider MEE's Aims and Scope for your manuscript. We make no guarantee that your manuscript will be within MEE scope.)
  • (Although not required, we strongly recommend having a full manuscript prepared when you submit here.)
  • (Please do not submit your package separately to Methods in Ecology and Evolution)

Code of conduct

@maelle
Copy link
Member

@maelle maelle commented Nov 4, 2019

👋 @MohammedFCIS thanks for your submission! An editor will soon be assigned to the submission, thanks for your patience.

@noamross
Copy link
Collaborator

@noamross noamross commented Nov 8, 2019

Hello @MohammedFCIS, thank for your submission, I'll be your handling editor.

Editor checks:

  • Fit: The package meets criteria for fit and overlap
  • Automated tests: Package has a testing suite and is tested via Travis-CI or another CI service.
  • License: The package has a CRAN or OSI accepted license
  • Repository: The repository link resolves correctly
  • Archive (JOSS only, may be post-review): The repository DOI resolves correctly
  • Version (JOSS only, may be post-review): Does the release version given match the GitHub release (v1.0.0)?

Editor comments

Here is output from goodpractice::gp(). Test coverage is very good - reviewers should use covr::report() to inspect uncovered areas. Use of <- or = is up the author, but should be consistent througout the package. Consider line length on a case-by-case basis, but consider the styler package or something similar for code formatting.

The are all minor and can be addressed as I look for reviewers, which I am doing now!

── GP dbparser ─────────────────────────

It is good practice to

  ✖ write unit tests for all functions, and all package code  
    in general. 91% of code lines are covered by test cases.

    R/drug_atc_codes_node_parser.R:74:NA
    R/drug_carrier_node_parser.R:26:NA
    R/drug_carrier_node_parser.R:27:NA
    R/drug_carrier_node_parser.R:28:NA
    R/drug_carrier_node_parser.R:29:NA
    ... and 186 more lines

  ✖ omit "Date" in DESCRIPTION. It is not required and it
    gets invalid quite often. A build date will be added to the package
    when you perform `R CMD build` on it.
  ✖ use '<-' for assignment instead of '='. '<-' is the
    standard, and R users and developers are used it and it is easier
    to read your code for them if you use '<-'.

    R/drug_parsers.R:986:39
    R/drug_polypeptide_sub_node_parser.R:3:13

  ✖ avoid long code lines, it is bad for readability. Also,
    many people prefer editor windows that are about 80 characters
    wide. Try make your lines shorter than 80 characters

    R/dbparser.R:1:1
    R/dbparser.R:7:1
    R/dbparser.R:13:1
    R/drug_all_nodes_parser.R:14:1
    R/drug_all_nodes_parser.R:15:1
    ... and 587 more lines

  ✖ avoid sapply(), it is not type safe. It might return a
    vector, or a list, depending on the input data. Consider using
    vapply() instead.

    tests/testthat/test_drug_all_nodes_parser.R:10:3

───────────────────────────────────────
---


Reviewers:
Due date:
@MohammedFCIS
Copy link
Author

@MohammedFCIS MohammedFCIS commented Nov 10, 2019

Hello @noamross , I have fixed all goodpractice notes, except test coverage that increased to 92%, is it necessary to increase the test coverage to 100% before resuming review? Thank you

@noamross
Copy link
Collaborator

@noamross noamross commented Nov 18, 2019

Thank you @MohammedFCIS. No, 100% test coverage is not required.

Thank you @emmamendelsohn and @haozhu233 for agreeing to review! Reviews are due 2019-12-09.

@emmamendelsohn
Copy link

@emmamendelsohn emmamendelsohn commented Dec 2, 2019

Hi @MohammedFCIS! I'm in the process of acquiring the DrugBank dataset--I just responded to a request for more information about how I will use the data. Hopefully they will approve me soon! In the meantime, I was wondering if you considered using the DrugBank API? Could it potentially provide a smoother path for users to access the data?

@MohammedFCIS
Copy link
Author

@MohammedFCIS MohammedFCIS commented Dec 4, 2019

Hi @emmamendelsohn , first let me thank you so much for taking time and effort to review our package.
As for the DrugBank database I have the following backup in my google drive and you can use it if you like :)
https://drive.google.com/open?id=1Em-IYnuHeOENE9qoiZxX2QGr9xE2fS_r.

Regarding DrugBank API, we considered using it at first of course, but decided to proceed with our dbparser for the following reasons:

  • We needed DrugBank database parser to parse any piece of information we want, not just those available on the DrugBank API.
  • We needed to use these information as R tibbles for developed drugs development machine learning algorithms.
  • Moreover, we decided to build a complete ecosystem system for drugs development https://dainanahan.github.io/drugverse/, and dbparser is just the begining ;).
  • We wanted to make a contribution to open science community as general.
  • I am not sure if those API's are free as well :)

I hope that answered your question :). Thank you.

@noamross
Copy link
Collaborator

@noamross noamross commented Dec 9, 2019

Hello @emmamendelsohn and @haozhu233, a reminder that your reviews are due today.

@haozhu233
Copy link

@haozhu233 haozhu233 commented Dec 9, 2019

Hi, all,

Here is my review. @MohammedFCIS Good job on this package! I totally enjoyed reviewing it! :)

Hao


Package Review

Please check off boxes as applicable, and elaborate in comments below. Your review is not limited to these topics, as described in the reviewer guide

  • As the reviewer I confirm that there are no conflicts of interest for me to review this work (If you are unsure whether you are in conflict, please speak to your editor before starting your review).

Documentation

The package includes all the following forms of documentation:

  • A statement of need clearly stating problems the software is designed to solve and its target audience in README
  • Installation instructions: for the development version of package and any non-standard dependencies in README
  • Vignette(s) demonstrating major functionality that runs successfully locally
  • Function Documentation: for all exported functions in R help
  • Examples for all exported functions in R Help that run successfully locally
  • Community guidelines including contribution guidelines in the README or CONTRIBUTING, and DESCRIPTION with URL, BugReports and Maintainer (which may be autogenerated via Authors@R).
For packages co-submitting to JOSS

The package contains a paper.md matching JOSS's requirements with:

  • A short summary describing the high-level functionality of the software
  • Authors: A list of authors with their affiliations
  • A statement of need clearly stating problems the software is designed to solve and its target audience.
  • References: with DOIs for all those that have one (e.g. papers, datasets, software).

Functionality

  • Installation: Installation succeeds as documented.
  • Functionality: Any functional claims of the software been confirmed.
  • Performance: Any performance claims of the software been confirmed.
  • Automated tests: Unit tests cover essential functions of the package
    and a reasonable range of inputs and conditions. All tests pass on the local machine.
  • Packaging guidelines: The package conforms to the rOpenSci packaging guidelines

Final approval (post-review)

  • The author has responded to my review and made changes to my satisfaction. I recommend approving this package.

Estimated hours spent reviewing: 4 hr

  • Should the author(s) deem it appropriate, I agree to be acknowledged as a package reviewer ("rev" role) in the package DESCRIPTION file.

Review Comments

The Drugbank database provides quite some useful information for the bioinformatics and cheminformatics research. However, for new researchers, especially for those who are not familiar with the xml format, it might be a little difficult for them to get their hands on the data. The dbparser package provides a set of functions to parse the drugbank xml data file. I think as a data extraction tool, it achieves its purpose and can bring a lot of convenience to its users. The code in this package are well written and documented with a few exceptions, which is listed below.

Some notes and observations:

  1. I noticed that right now DrugBank is released under version 5 (5.1.4 as of today). I would imagine there could be inconsistencies in the database between each major release. I wonder if this package can be used to read data from DrugBank version 4.0 or 6.0 without getting an error. Have you checked what kind of changes they made for every major release? If this package only works with version 5, it might be a good idea to note it in DESCRIPTION or README, or add a database version check in get_xml_db_rows.

  2. rOpenSci strongly recommend xml2 for parsing xml data whereas this package uses XML. I guess at this point it might be a little too late and overkill to ask you to switch to xml2 but hopefully you could consider to use that next time. :)

See https://devguide.ropensci.org/building.html#recommended-scaffolding

  1. I noticed that you tried to fix some function names that are too long a few weeks ago. It's generally a good idea to keep variable names short but I think your fixes brought up some inconsistencies among functions under the same umbrella. For example, all the functions in the screenshot below work with drug carrier data. Why do some functions start with parse_carr while others start with parse_drug_carriers? In fact, I think users of this package already know that they are tying to parse something. Do they really need to type parse_ or parse_drug_ every time when they want to get some data? How about replacing it with something like db_ (or something more appropriate).

Screen Shot 2019-12-09 at 1 24 54 PM

See https://devguide.ropensci.org/building.html#function-and-argument-naming

  1. get_xml_db_rows seems to be the name of a secondary function but is in fact the initial step of reading in the data.

  2. Should this file existence check take place at the very beginning?
    https://github.com/Dainanahan/dbparser/blob/b4d4aa47dab24e4eb291eec96ba5f7bbeb4275ef/R/drug_common_utilities.R#L96

  3. I think the "Exploring the data" section is a little too heavy for README. This kind of contents is great in the getting started vignette but is not that useful in README as it is basically demonstrating dplyr and ggplot.

Package check passed without warning/notes on my machine.

@emmamendelsohn
Copy link

@emmamendelsohn emmamendelsohn commented Dec 10, 2019

@MohammedFCIS Nice job and thanks for the opportunity to review. Happy to discuss/answer questions.

Package Review

Please check off boxes as applicable, and elaborate in comments below. Your review is not limited to these topics, as described in the reviewer guide

  • As the reviewer I confirm that there are no conflicts of interest for me to review this work (If you are unsure whether you are in conflict, please speak to your editor before starting your review).

Documentation

The package includes all the following forms of documentation:

  • A statement of need clearly stating problems the software is designed to solve and its target audience in README
  • Installation instructions: for the development version of package and any non-standard dependencies in README
  • Vignette(s) demonstrating major functionality that runs successfully locally
  • Function Documentation: for all exported functions in R help
  • Examples for all exported functions in R Help that run successfully locally
  • Community guidelines including contribution guidelines in the README or CONTRIBUTING, and DESCRIPTION with URL, BugReports and Maintainer (which may be autogenerated via Authors@R).

Functionality

  • Installation: Installation succeeds as documented.
  • Functionality: Any functional claims of the software been confirmed.
  • Performance: Any performance claims of the software been confirmed.
  • Automated tests: Unit tests cover essential functions of the package
    and a reasonable range of inputs and conditions. All tests pass on the local machine.
  • Packaging guidelines: The package conforms to the rOpenSci packaging guidelines

Final approval (post-review)

  • The author has responded to my review and made changes to my satisfaction. I recommend approving this package.

Estimated hours spent reviewing: 4

  • Should the author(s) deem it appropriate, I agree to be acknowledged as a package reviewer ("rev" role) in the package DESCRIPTION file.

Review Comments

dbparser is a convenient package for parsing XML DrugBank data into tidy format. I have a few minor comments on documentation, and then a more general question about saving the data into a database.

Tests run fine with devtools::test(). Running testthat.R yields an error: Error: No tests found for dbparser.

notes on README

  • I think it would help unfamiliar users to add another sentence in the introduction explaining what DrugBank is. It would also be helpful to include a note about the need to make an account and request access to the data.
  • Suggest removing commented code
  • It seems like you made some changes recently that are resulting in errors in the README, so I suggest combing through it to make sure it's working as expected. Below are a couple things I've noticed.
    • parse_drug_targets_actions() no longer works (it's now parse_drug_targ_actions()). Side note: I agree with @haozhu233 it would be nice to shorten some of the function names if possible.
    • xml_db_name is not an argument in open_db()
  • As expected parse_drug(save_table = TRUE) (and all parse functions) throws an error when there is no database to save to. However, the error only happens after parsing the data for about a minute, so it would be nice to throw an early informative error to the user.
  • Make sure all relevant libraries are called for the plots (ggplot2, dplyr)
  • The package description in R help can use a bit of clarification. I'd suggest emphasizing the drug bank parser over the database functionality.

Database

This may be due to inexperience on my part, but I was confused about how to save the database using the driver and server argument. I would have liked to be able to save it to a SQLite or other local database using DBI::dbConnect. Is it possible to make these functions more generalizable? Or at minimum, add more description of what type of databases the package supports?

@MohammedFCIS
Copy link
Author

@MohammedFCIS MohammedFCIS commented Dec 10, 2019

Hello @haozhu233 and @emmamendelsohn ;
Thank you so much for your time and effort that you are giving for this review.
I did not only benefited from your comments but I enjoyed your feedback so much as well, thanks for that also.
Kindly find my detailed response below:

I noticed that right now DrugBank is released under version 5 (5.1.4 as of today). I would imagine there could be inconsistencies in the database between each major release. I wonder if this package can be used to read data from DrugBank version 4.0 or 6.0 without getting an error. Have you checked what kind of changes they made for every major release? If this package only works with version 5, it might be a good idea to note it in DESCRIPTION or README, or add a database version check in get_xml_db_rows.

I agree with you, the following issue has been created for that.
ropensci/dbparser#73

rOpenSci strongly recommend xml2 for parsing xml data whereas this package uses XML. I guess at this point it might be a little too late and overkill to ask you to switch to xml2 but hopefully you could consider to use that next time. :)

Actually it is in my plan to migrate to xml2, kindly check that issue:
ropensci/dbparser#63

I noticed that you tried to fix some function names that are too long a few weeks ago. It's generally a good idea to keep variable names short but I think your fixes brought up some inconsistencies among functions under the same umbrella. For example, all the functions in the screenshot below work with drug carrier data. Why do some functions start with parse_carr while others start with parse_drug_carriers? In fact, I think users of this package already know that they are tying to parse something. Do they really need to type parse_ or parse_drug_ every time when they want to get some data? How about replacing it with something like db_ (or something more appropriate)

The following issue is created to fix that ropensci/dbparser#74

get_xml_db_rows seems to be the name of a secondary function but is in fact the initial step of reading in the data.

The following issue is created to fix that ropensci/dbparser#75

Should this file existence check take place at the very beginning?
https://github.com/Dainanahan/dbparser/blob/b4d4aa47dab24e4eb291eec96ba5f7bbeb4275ef/R/drug_common_utilities.R#L96

It is in the beginning already, nothing before it but zip file check. As if the file we have is a zip file we must unzip before checking for the db file.

I think the "Exploring the data" section is a little too heavy for README. This kind of contents is great in the getting started vignette but is not that useful in README as it is basically demonstrating dplyr and ggplot.

Added this issue to fix it ropensci/dbparser#14
that to cover @emmamendelsohn notes as well.
I will start working on your notes right away.
Best Regards,

@noamross
Copy link
Collaborator

@noamross noamross commented Dec 12, 2019

Thank you for your reviews, @haozhu233 and @emmamendelsohn. Thank you for responding quickly @MohammedFCIS and we look forward to the revision when you have addressed all the comments. I concur with @emmamendelsohn's comment about the database - I'm not sure if there is a specific reason that MySQL is used, but I think it would make more sense to just let the user use any back-end database they pass. I think you're already doing this.

@MohammedFCIS
Copy link
Author

@MohammedFCIS MohammedFCIS commented Jan 6, 2020

Hello @haozhu233 and @emmamendelsohn. Happy new year. Kindly find the required changes here https://github.com/Dainanahan/dbparser/labels/ropensci.
Please let me know if you have any further notes. Thank you.
Best Regards,

@noamross
Copy link
Collaborator

@noamross noamross commented Jan 7, 2020

Thank you @MohammedFCIS! @emmamendelsohn and @haozhu233, please let us know if the changes address your comments.

@emmamendelsohn
Copy link

@emmamendelsohn emmamendelsohn commented Jan 18, 2020

Hi @MohammedFCIS! Nice job with the updates--the package feels much more polished. I think the new function names work well.

I like that you shortened the README and made a separate tutorial. I'm submitting a PR with a few suggested changes to the README text. Feel free to use or discard as you wish :)

I have just a few comments on this second review. Let me know if you need any clarification.

Database capabilities

  • I see that you added a vignette on this. Can you link to it from the README and tutorial so that the user can easily locate it?
  • Echoing my comments from the first review: is it necessary for the user to connect to a server? What if the user wants to save to a local DBI connection?

Tutorial

  • I would add something to the top saying that it assumes you have already downloaded the data (per instructions in the README), saved it to your working directory, and renamed it to drugbank.xml.
  • Show options to save data in your functions (e.g., drug(save_csv=TRUE)).
  • The function hyperlinks are broken.

Dataframes versus tibbles

  • Sorry I missed this the first time: some functions return dataframes (e.g., drug_articles()) but most return tibbles. I think from your package description you intend for everything to return tibbles, so you may need to do a sweep to make sure all are tibbles. In addition, the reference page is inconsistent about reporting tibble versus dataframe, so it should be updated too.

Parsers

  • When running a function without first running read_drugbank_xml_db("drugbank.xml"), an empty tibble is returned. Maybe add a message reminding users to first run the read function?

R help

  • I know it's not a major point of entry to the package, but I'd suggest updating the ?dbparser text to match some of the language you developed in the README etc.
@MohammedFCIS
Copy link
Author

@MohammedFCIS MohammedFCIS commented Jan 18, 2020

Hello @emmamendelsohn ,
As usual I am enjoying your notes and your contribution is very welcome and appreciated of course :).
I have created issues with your notes here https://github.com/Dainanahan/dbparser/labels/ropensci and I will work on it immediately.
I just need clarifications about these two points:

  • Show options to save data in your functions (e.g., drug(save_csv=TRUE)) ==> Show it where as it is written in the documentation?
  • The function hyperlinks are broken ==> (what functions links are broken).
    For DBI part I will clarify it more on the tutorial, but user can use anything that DBI support including local db.
    Thank you.
@emmamendelsohn
Copy link

@emmamendelsohn emmamendelsohn commented Jan 20, 2020

  • Yes, a very minor suggestion: you may want to mention in the tutorial that the user has the option to set save_csv=TRUE in the parse functions.
  • Hmm, this may actually be beyond your control? When I view the tutorial, all the functions are hyperlinks. Clicking on drug() takes me to this link, which is not found: https://rdrr.io/cran/dbparser/man/drug.html. I'm guessing it has to do with the GitHub version falling out of sync with CRAN?
@MohammedFCIS
Copy link
Author

@MohammedFCIS MohammedFCIS commented Jan 20, 2020

Thank you @emmamendelsohn for clarifications.
For the links point, I think you are right it is out of my hand. What I am using are the references from package site here https://dainanahan.github.io/dbparser/reference/index.html and https://dainanahan.github.io/dbparser/reference/drug.html

@MohammedFCIS
Copy link
Author

@MohammedFCIS MohammedFCIS commented Jan 22, 2020

Hello @emmamendelsohn ,
I finished your notes, kindly check here https://github.com/Dainanahan/dbparser/labels/ropensci.
Thank you.

@MohammedFCIS
Copy link
Author

@MohammedFCIS MohammedFCIS commented Jan 30, 2020

Hello @emmamendelsohn , @haozhu233 and @noamross,
Is there any thing still required from me regarding this request? Thank you so much :)

@emmamendelsohn
Copy link

@emmamendelsohn emmamendelsohn commented Jan 31, 2020

Thanks for the reminder, @MohammedFCIS. I reviewed your responses to my comments and everything looks good. I just submitted a PR with some edits to the database vignette. @noamross it would be helpful if you could give the vignette a read-through as well.

@noamross
Copy link
Collaborator

@noamross noamross commented Jan 31, 2020

I'm trying to get in touch with our second reviewer. If I don't hear from him I will go through on his behalf.

@haozhu233
Copy link

@haozhu233 haozhu233 commented Feb 1, 2020

Hi, all, sorry for being a little late on getting back on this. I had been quite busy recently.

@MohammedFCIS I was actually amazed by the amount of updates you made since I reviewed it last time. Right now this package looks very polished and I would say the software interface looks very clean right now and very user friendly! Impressive!

After reviewing this new version several times, I only have a tiny comment regarding the use of %>%. I noticed that you imported pipe from tidyr and that is the only thing you imported from that package. This is a little weird. You can either imported it from magrittr or from purrr, which also has %>% exported.

@MohammedFCIS
Copy link
Author

@MohammedFCIS MohammedFCIS commented Feb 2, 2020

Thank you so much @haozhu233 for your feedback :). It is only due to your and @emmamendelsohn very useful notes ;).
I have fixed your note here ropensci/dbparser#85
Best Regards,

@noamross
Copy link
Collaborator

@noamross noamross commented Feb 4, 2020

Thank you @emmamendelsohn and @haozhu233 for your responses. I am doing final editor checks and have a an issue on the database vignette / open_db() function and documentation. The function is hard-coded for either SQLite or an ODBC database, but this isn't clear in the vignette, which only refers to "DBI-compliant" databases. While this could be fixed by documentation, I think it would be simpler for instead the open_db() function to simply take a connection as its first argument, so you don't have to hard-code any of these options. That is, open_db(RSQLite::RSQLite(...)) passes this connection to pkg_env$con. The default or open_mdb can still go to MariaDB.

@MohammedFCIS
Copy link
Author

@MohammedFCIS MohammedFCIS commented Feb 14, 2020

Thank you @noamross so much for your valuable note. It inspired me to change a lot actually. Please find the fix in that issue ropensci/dbparser#87.
I have updated both the code, documentation and tutorial.
@haozhu233, @emmamendelsohn please feel free to review this update as well.
Thank you all for your time and valuable input.

@noamross
Copy link
Collaborator

@noamross noamross commented Feb 18, 2020

Thank you, @MohammedFCIS, for the update. The package is approved! Thanks for submitting and @emmamendelsohn and @haozhu233 for your reviews!

To-dos:

  • Transfer the repo to rOpenSci's "ropensci" GitHub organization under "Settings" in your repo. I have invited you to a team that should allow you to do so. You'll be made admin once you do.
  • Fix all links to the GitHub repo to point to the repo under the ropensci organization.
  • If you already had a pkgdown website, fix its URL to point to https://docs.ropensci.org/package_name and deactivate the automatic deployment you might have set up, since it will not be built centrally like for all rOpenSci packages, see http://devdevguide.netlify.com/#docsropensci. In addition, in your DESCRIPTION file, include the docs link in the URL field alongside the link to the GitHub repository, e.g.: URL: https://docs.ropensci.org/foobar (website) https://github.com/ropensci/foobar
  • Fix any links in badges for CI and coverage to point to the ropensci URL.
  • We're starting to roll out software metadata files to all ropensci packages via the Codemeta initiative, see https://github.com/ropensci/codemetar/#codemetar for how to include it in your package, after installing the package - should be easy as running codemetar::write_codemeta() in the root of your package.

Should you want to acknowledge your reviewers in your package DESCRIPTION, you can do so by making them "rev"-type contributors in the Authors@R field. More info on this here.

We've put together an online book with our best practice and tips, this chapter starts the section that's about guidance for post-review. Please tell us what could be improved, the corresponding repo is here.

@karthik karthik added the package label Feb 18, 2020
@MohammedFCIS
Copy link
Author

@MohammedFCIS MohammedFCIS commented Feb 18, 2020

Hi @noamross . Thank you for the great news, I transferred the repo and waiting to be admin on it to continue the remaining steps. Thank you

@noamross
Copy link
Collaborator

@noamross noamross commented Feb 18, 2020

You should be all set.

@MohammedFCIS
Copy link
Author

@MohammedFCIS MohammedFCIS commented Feb 18, 2020

Thanks a lot @noamross :)

@MohammedFCIS
Copy link
Author

@MohammedFCIS MohammedFCIS commented Feb 19, 2020

Hi @noamross, the following ToDo list is finished.

  • Transfer the repo to rOpenSci's "ropensci" GitHub organization under "Settings" in your repo. I have invited you to a team that should allow you to do so. You'll be made admin once you do.
  • Fix all links to the GitHub repo to point to the repo under the ropensci organization.
    If you already had a pkgdown website, fix its URL to point to https://docs.ropensci.org/package_name and deactivate the automatic deployment you might have set up, since it will not be built centrally like for all rOpenSci packages, see http://devdevguide.netlify.com/#docsropensci. In addition, in your DESCRIPTION file, include the docs link in the URL field alongside the link to the GitHub repository, e.g.: URL: https://docs.ropensci.org/foobar (website) https://github.com/ropensci/foobar
  • Fix any links in badges for CI and coverage to point to the ropensci URL.
  • Adding reviewers to package descriptions.

Thank you

@noamross
Copy link
Collaborator

@noamross noamross commented Feb 19, 2020

Great work!

@noamross noamross closed this Feb 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
7 participants
You can’t perform that action at this time.