search

UBC-MDS / software-review-2022

0 stars 0 forks source link

Submission Group 30: chemcalculatorrr (R) #10

Open Kingslin0810 opened 2 years ago

Kingslin0810 commented 2 years ago

Submitting Author Name: Kingslin Lv, Joyce Wang, and Allyson Stoll Submitting Author Github Handle: @Kingslin0810, @jo4356, and @datallurgy Repository: chemcalculatorrr Version submitted: 1.0.0 Submission type: Standard Editor: Florencia D'Andrea (@flor14) Language: en

The package chemcalculatorrr converts between moles and grams for chemical formulas, helping researchers or data scientist to perform this widely used conversions fast and without errors..

Researchers or data scientists working with chemistry data.

No. This package of basic chemistry calculations is meant to supplement an existing package, ChemPy, which already handles complex calculations for primarily physical/inorganic/analytical chemistry consisting of, but not limited to, the following:

  • Solver for equilibria (including multiphase systems)
  • Numerical integration routines for chemical kinetics (ODE solver front-end)
  • Integrated rate expressions (and convenience fitting routines)
  • Relations in Physical chemistry
  • Debye-Hückel expressions
  • Arrhenius equation
  • Einstein-Smoluchowski equation
  • Properties, such as : water density as function of temperature, water permittivity as function of temperature and pressure, and water diffusivity as function of temperature

Technical checks

Confirm each of the following by checking the box.

This package:

Publication options

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 * [x] I agree to abide by [rOpenSci's Code of Conduct](https://ropensci.org/code-of-conduct/) during the review process and in maintaining my package should it be accepted.
khbunyan commented 2 years ago

Package Review

Documentation

The package includes all the following forms of documentation:

Functionality

Estimated hours spent reviewing: 1.5

Review Comments

Hi group 30, well done on this package! Similar to my comments for your python package, your repo is clean, concise, presents a great tool for chemists, and seems as though you are the one of the first to create something like this in the R space. This project is really well put together; everything is clearly organized, mostly everything required is included and easy to find, and the written documentation is very well organized so it's simple to follow.

I have a few notes on some things I saw in your project that you could explore for improvement. Some of these are the same comments from your python package as the same things existed here:

  1. In the argument -> formula section of the Roxygen documentation for moles_grams_converter() there is a small spelling mistake: "A checmical formula for the conversion"
  2. In the Value section of the Roxygen documentation for moles_grams_converter() there is a small grammar mistake, a "for" is missing: "A number FOR mass that is converted to either moles or grams"
  3. In the Arguments and Value sections of the Roxygen documentation for compute_mass() there are periods ending the sentences. I believe official Roxygen style is to leave them without a period.
  4. A small spelling error in the README description: "same function as the python pacakge."
  5. The README still references the python material, this may have been left in by mistake : "This package of basic chemistry calculations is meant to supplement an existing package, ChemPy, etc."
  6. The installation instructions, as they are right now, do not allow you to download the package because the package is not accessible this way: "install.packages("chemcalculatorrr")". I would update this section to describe the steps one needs to take to get access to and use the package. I personally cloned the repo, created an R project with it, and then used load_all().
  7. I think moles_grams_converter() might not be performing what is intended here, similar to in your python package.
    • Looking at the Roxygen documentation the input convert_to is "The type of conversion to be made to either "moles" or "grams"", however in your test function it shows this:
    • Screen Shot 2022-01-31 at 10 35 49 PM
    • in order for the result of 1 to be true, the 18.015 value would need to represent grams and the result of 1 would be mols.
    • However the input for convert_to is "grams", so I do not think this is giving the result that is expected based on the Roxygen documentation. convert_to currently represents what the "value" is pre-conversion, not post, it's what it's converting from.
    • Screen Shot 2022-01-31 at 10 41 32 PM
    • In your second test pictured above, it shows that you should get a result of 18.015 and are converting moles to grams. However, when I run this on my machine I get 0.056. 1 gram represents 0.056 mols, so again I think convert_to is mistakenly representing the units we are converting from.

In addition to the above comments, there are some missed check boxes above that you could review as well. All of your tests passed with no notes or warnings, as well as the covr coverage test, which is excellent. Again, great work everyone, please reach out if you have any questions. Thank you

berkaybulut commented 2 years ago

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

Documentation

The package includes all the following forms of documentation:

Functionality

Estimated hours spent reviewing: 2

Review Comments

Hello group 30, congratulations on your repository. It is a unique repo that is focused on chemists in R space, and it is one of the first that I have come across something like this. I think your repository is well organized and easy to understand. It is well put together and simple to follow.

I have a few comments that you can work on to improve further.

  1. You can separate each function to have its own unique script file for future upgradeability and reduction in merge conflicts.

  2. The installation instructions are not working. I currently cannot install package using "install.packages("chemcalculatorrr")". I think this can be updated to include necessary steps. You can list all the steps by using git to install package.

  3. There are 4 contributors in your repository but in your Readme.md you have listed 3 contributors as contributor developers.

  4. In the Roxygen of function moles_grams_converter function there is a small spelling mistake: "A checmical formula for the conversion"

  5. In the Roxygen documentation for compute_mass() sentences end with periods. Roxygen suggested style is to leave them without a period.

All of your tests passed including coverage test. Great work to everyone - let me know if you have any questions!

jo4356 commented 2 years ago

@berkaybulut Thanks for your review! To address some of your comments:

  1. We have already updated the installation instructions to use install_github("UBC-MDS/chemcalculatorrr") instead of install.packages
  2. We only have 3 contributions. Two of the account listed as contributors belongs to the same person.
Kingslin0810 commented 2 years ago

thanks @berkaybulut some typos are fixed here

helingogo commented 2 years ago

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

Documentation

The package includes all the following forms of documentation:

Functionality

Estimated hours spent reviewing: 1.5

Review Comments

After adding Vignettes parts and updating your documentation, I think your repo looks fantastic! As a business background person who have very limited chemistry knowledge, I can still easily understand your functions by reading through the documentation. The structure looks very clean and organized. Each file resides in the right folder. The package can be successfully installed and functions also works well. The test coverage also goes to 95.55%. Congratulations on completion of this project!

Here are some problems and suggestions:

  1. I see that you keep 3 decimals for the outcome. However, in the help file, the examples part shows outcome of different number of decimals. I think you can update the document.

  2. Also, the outcome in help file for compute_mass("H2O") example is different from the actual number which returns 18.013. Same problem happens to percent_mass().

  3. As a suggestion, you can separate the functions into different R file, since it will be more flexible to make modification and update on each function, and it will reduce conflicts to a big extent when making PR and merging contribution.

  4. Package dependencies and License can be added to Readme file.

  5. It is better to write library with the function. For example, use here::here(...) in the code instead of putting `library(here) on the top.

Congrats again for finishing the repo. Let me know if you have any question.