coinmarketcapr

[amrrs]

Summary

• What does this package do? (explain in 50 words or less):

R package to get Cryptocurrencies Market Cap Prices from Coin Market Cap

• Paste the full DESCRIPTION file inside a code block below:

Package: coinmarketcapr
Type: Package
Title: Get Cryptocurrencies Market Cap Prices from Coin Market Cap
Version: 0.1
Date: 2017-09-24
Authors@R: c(person("AbdulMajedRaja","RS", email = "amrrs.data@gmail.com", role = "cre"), person("Srivathsan","K", email = "srivibish@gmail.com", role = "aut"))
Description: To extract and monitor price and market cap of 'Crypto currencies' from 'Coin Market Cap' <https://coinmarketcap.com/api/>. 
URL: http://github.com/amrrs/coinmarketcapr
License: CC0
LazyData: true
Imports: jsonlite,
    ggplot2,
    RCurl
Suggests: testthat
RoxygenNote: 6.0.1

• URL for the package (the development repository, not a stylized html page):
  https://github.com/amrrs/coinmarketcapr

• Please indicate which category or categories from our package fit policies this package falls under *and why(? (e.g., data retrieval, reproducibility. If you are unsure, we suggest you make a pre-submission inquiry.): database access, because the package is wrapper around coinmarketcap api to extract cryptodata

[e.g., "data extraction, because the package parses a scientific data file format"]

•   Who is the target audience and what are scientific applications of this package?  
  R programmers who like to analyse Cryptocurrencies

• Are there other R packages that accomplish the same thing? If so, how does
  yours differ or meet our criteria for best-in-category? No (yet)

•   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.

Requirements

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

• does not violate the Terms of Service of any service it interacts with.
• has a CRAN and OSI accepted license.
• contains a README with instructions for installing the development version.
• includes documentation with examples for all functions.
• contains a vignette with examples of its essential functions and uses.
• has a test suite.
• has continuous integration, including reporting of test coverage, using services such as Travis CI, Coeveralls and/or CodeCov.
• I agree to abide by ROpenSci's Code of Conduct during the review process and in maintaining my package should it be accepted.

Publication options

• Do you intend for this package to go on CRAN? - already on CRAN
• Do you wish to automatically submit to the Journal of Open Source Software? If so:
  • 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)
• Do you wish to submit an Applications Article about your package to Methods in Ecology and Evolution? If so:
  • 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.
  • (Please do not submit your package separately to Methods in Ecology and Evolution)

Detail

• Does R CMD check (or devtools::check()) succeed? Paste and describe any errors or warnings:

• Does the package conform to rOpenSci packaging guidelines? Please describe any exceptions:

• If this is a resubmission following rejection, please explain the change in circumstances:

• If possible, please provide recommendations of reviewers - those with experience with similar packages and/or likely users of your package - and their GitHub user names:

[amrrs]

@karthik Just checking, Is something happening regarding it or Has it been discarded?

[karthik]

@amrrs Sorry for the delay. The editors are just back from break. We'll update the thread shortly.

[amrrs]

That's great. Will wait for the response. On 10 Jan 2018, at 00:03, Karthik Ram <notifications@github.com<mailto:notifications@github.com>> wrote: @amrrs<https://github.com/amrrs> Sorry for the delay. The editors are just back from break. We'll update the thread shortly. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub<#172 (comment)>, or mute the thread<https://github.com/notifications/unsubscribe-auth/AFGX-vBZXTcRq12VvDbv3FOgfwuovUxBks5tI7D6gaJpZM4RF2FW>.

[noamross]

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

Thank you for your submission, @amrrs! A few things to address before we send this to review:

• We do require a vignette
• We require automated test coverage reporting with a badge (see usethis::use_coverage()).
• I also note that, while CC0 is an acceptable CRAN license, it's not OSI compatible, so unless you have a specific reason requiring you to use CC0, we suggest an MIT license.
• I note that your documentation is rather sparse. We'd like documentation to be complete so reviewers can comment on it. Note this guideline from our packaging guide:

If your package connects to a data source or online service, or wraps other software, consider that your package README may be the first point of entry for users. It should provide enough information for users to understand the nature of the data, service, or software, and provide links to other relevant data and documentation. For instance, a README should not merely read, "Provides access to GooberDB," but also include, "..., an online repository of Goober sightings in South America. More information about GooberDB, and documentation of database structure and metadata can be found at link.

We suggest his approach pervade all your documentation.

• Below is output from goodpractice::gp() diagnostics. I note your code coverage is 25%. This is low. We don't require 100% coverage but we want all major functionalities of the package covered, and reviewers can look at whether there are issues with coverage of edge cases. The other things are mostly small and easily addressable. Line length and package imports issues are guidelines; reviewers may opine on how necessary these changes are for your package.

── GP coinmarketcapr ──────────────────────────────────

It is good practice to

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

    R/coinmarketcapr.R:33:NA
    R/coinmarketcapr.R:35:NA
    R/coinmarketcapr.R:51:NA
    R/coinmarketcapr.R:53:NA
    R/coinmarketcapr.R:55:NA
    ... and 1 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.
  ✖ add a "BugReports" field to DESCRIPTION, and point it to a bug tracker. Many
    online code hosting services provide bug trackers for free, https://github.com,
    https://gitlab.com, etc.
  ✖ 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/coinmarketcapr.R:1:1
    R/coinmarketcapr.R:17:1
    R/coinmarketcapr.R:35:1
    R/coinmarketcapr.R:53:1
    R/coinmarketcapr.R:59:1

  ✖ not import packages as a whole, as this can cause name clashes between the
    imported packages. Instead, import only the specific functions you need.
───────────────────────────────────────────────

---

Reviewers: @sokal1456 @kaneplusplus
Due date: 2018-02-190

[amrrs]

@noamross Thanks for the comments.

• Fixed the coverage
• Started writing Vignette
• Will change the license
• Reg. the documentation, Is it fine if I just add more description around the API service in Readme.md or is there anywhere else I've got to add details?

[noamross]

You don't have to put verbose information everywhere, but we find it helpful to put links back to wherever you have the most comprehensive description in the vignette and in top-level package documentation (?coinmarketcapr).

[amrrs]

@noamross I tried fixing the essentials you mentioned. Could you please check it now?

[noamross]

Thank you for the updates, @amrrs. I am now seeking reviewers. Here is the updated goodpractice::gp() output. None is critical, though I suggest you add the BugReports: field before this process is over.

── GP coinmarketcapr ─────────────────────────────────

It is good practice to

  ✖ add a "BugReports" field to DESCRIPTION, and point
    it to a bug tracker. Many online code hosting services provide
    bug trackers for free, https://github.com, https://gitlab.com,
    etc.
  ✖ 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/coinmarketcapr.R:1:1
    R/coinmarketcapr.R:17:1
    R/coinmarketcapr.R:35:1
    R/coinmarketcapr.R:53:1
    R/coinmarketcapr.R:59:1

  ✖ not import packages as a whole, as this can cause
    name clashes between the imported packages. Instead, import
    only the specific functions you need.
─────────────────────────────────────────────

[amrrs]

Thanks @noamross . Sorry that's my bad not adding it, added bugreports now.

[noamross]

Thanks @sokal1456 and @kaneplusplus for agreeing to review! Due date is 2018-02-19.

[noamross]

@amrrs You can add our review badge to your README if you wish. It will read "under review" right now and updates with review status:

[![rOpenSci](https://badges.ropensci.org/172_status.svg)](https://github.com/ropensci/onboarding/issues/172)

@sokal1456 and @kaneplusplus You may be interested in using https://github.com/annakrystalli/pkgreviewr, an experimental reviewing workflow package.

[noamross]

Friendly reminders for both @sokal1456 and @kaneplusplus that reviews were due this week.

[kaneplusplus]

@noamross Thanks for the reminder. I should have it ready by the end of tomorrow.

[kaneplusplus]

@amrrs @noamross @sokal1456 This package provides a simple, minimal interface to coinmarketcap.com. I think this is exactly how these types of packages should be implemented. I have been using the package in conjunction with bittrex, to quickly discern active cryptos - with exhibit price seeking bahavior - from inactive ones with little to no volume.

I did run into a few issues with the implementation, which are described below. Fixes and revisions have been incorporated in this pull request. Each item is market as either mandatory or optional depending on its severity. I don't mind if the optional items get rolled back.

• (mandatory) Please don't start descriptions or titles with "To extract global market cap..."
• (mandatory) I've changed the plot_top_5_currencies() to be plot_top_currencies() to let the user decide how many currencies she/he wants to see.
• (mandatory) A few small typos.
• (optional) Excessive use of empty lines. An empty line helps the reader distinguish tasks within blocks of code. Too much is distracting.
• (optional) 8 spaces per tab. Tabs denote blocks of code that are within a scope. When tabs have too many spaces then you go over the recommended with of 80 characters quickly. I think going to 2 space tabs or even 4 space tabs increases the readability.

There were a couple of larger issues that also need to be fixed that were not included in my pull request. These will need to be addressed before the package is accepted.

1. Create a function that lists the valid currencies and update the vignette.
2. Add descriptions to the functions. You've written a package that assumes an internet connection, goes to the coinmarketcap api using RCurl, retrieves JSON, and turns it into a data.frame. Can you tell us that the package does these things and why someone might be interested in doing it?
3. Make an itemized list in the @return of the get_marketcap_ticker_all() function and describe what each field means.
4. Can you come up with a better description for plot_top_5_currencies()? The function plot the prices (in the specified currency) of the top 5 currencies by market cap. This should be made clearer.
5. Axis labels in plot_top_5_currencies() should be added. The y-axis label should be "Price in USD" if the currency is set to USD. The x-axis should be "Cryptocurrency".
6. The documentation titles for the get_global_marketcap() and get_marketcap_ticker_all() should be more descriptive.

[noamross]

Thanks for your review, @kaneplusplus!

[amrrs]

Thank you very much @kaneplusplus for the review and for being so kind to send a PR! Those things have been merged already. And I'm looking into the rest of the 6 points you've mentioned. Will get back to you soon with the details and my comments.

[sokal1456]

@amrrs @noamross @kaneplusplus This is my very first package review (and apologies that this is late!) to give you fair warning :)

The usefulness of this package lies in it's simplicity which is awesome.

Things I like:

-the package functions have easy to understand and descriptive names
-clear results that I can incorporate in analysis

Documentation:

1. t would be very useful to clearly define all the currencies one can use when pulling the data.
2. it would be helpful to also explain the types of results that I should expect in this case dataframes and plots.
3. Clearly explain for the plotting function how the currencies are being ranked.

Functionality:

-it would be cool if the user could choose the type of plot/color for plot_top_currencies()
-It would be great if this could produce tibbles etc because I would love to use this within my analysis and I usually do it with the tidyverse.
-plot_top_currencies() y-axis could have $ signs to make it more readable.

[amrrs]

Thanks @sokal1456 for your reviews. Just collating all the points together.

• 1. Create a function that lists the valid currencies and update the vignette.
• 2. Add descriptions to the functions. You've written a package that assumes an internet connection, goes to the coinmarketcap api using RCurl, retrieves JSON, and turns it into a data.frame. Can you tell us that the package does these things and why someone might be interested in doing it?
• 3. Make an itemized list in the @return of the get_marketcap_ticker_all() function and describe what each field means.
• 4. Can you come up with a better description for plot_top_5_currencies()? The function plot the prices (in the specified currency) of the top 5 currencies by market cap. This should be made clearer.
• 5. Axis labels in plot_top_5_currencies() should be added. The y-axis label should be "Price in USD" if the currency is set to USD. The x-axis should be "Cryptocurrency".
• 6. The documentation titles for the get_global_marketcap() and get_marketcap_ticker_all() should be more descriptive.
• 7. Tt would be very useful to clearly define all the currencies one can use when pulling the data.
• 8. it would be helpful to also explain the types of results that I should expect in this case dataframes and plots.
• 9. Clearly explain for the plotting function how the currencies are being ranked.
• 10. it would be cool if the user could choose the type of plot/color for plot_top_currencies()
• 11. It would be great if this could produce tibbles etc because I would love to use this within my analysis and I usually do it with the tidyverse.
• 12. plot_top_currencies() y-axis could have $ signs to make it more readable.

[noamross]

Thanks @sokal1456 and @kaneplusplus for reviews. After @amrrs responds with changes, please use the review template for your responses.

A note, @amrrs. Some package authors prefer to avoid the dependencies associated with importing the tibble package to return tibbles, so one option is to still return a tibble just by setting the class of a returned data frame with class(x) <- c("tbl_df", "tbl", "data.frame"). This works if your data frame meets tibble requirements, (correct names, no rownames, only POSIXct dates, etc.).

[noamross]

@amrrs, do you have an update?

[amrrs]

Hi @noamross Sorry i was caught up with a lot of other things so couldn't make any progress in this. Picked this up again. I've made the suggested changes as a task list and updating it above. Is that okay?

[amrrs]

@kaneplusplus Sorry I couldn't understand your point 3, Could you please explain it? Currently it returns a dataframe so not sure what this itemized list is supposed to do? Like, replace the dataframe with an itemized list?

Update: Even the API documentation itself doesn't have any extra information about the columns, so not sure if it's required / how accurate i could define what API creators didn't define.

[noamross]

@amrrs That's OK. It's just what we expect. Once you have gotten through all the changes let us know for the reviewers to approve, or ask about uncertain ones as you have above.

[kaneplusplus]

Sorry, that wasn't so clear. In the documentation, can you make an itemized list describing each column of the data frame?

I have sympathy for bad API documentation but at the same time I think we should be able to provide some description of the things our functions return.

[amrrs]

@noamross Coinmarketcap has updated their API and I've to move the package to the new API before November. So it seems ideal to submit after that. Any thoughts?

[noamross]

Dear @amrrs, I must write to apologize - this issue fell through the cracks in our editor rotation, and I only just rediscovered it in a pipeline cleanup. I'm really quite sorry that I didn't reply to the above and this got lost. If you are still working on this, can you tell me your current status and whether you'd like to complete the review? If so I'll make sure we expedite picking up the process.

[amrrs]

Hi @noamross That's completely fine. So kind of you to get back. The package is working as it should be, I guess I did fix most of the previous review's comments. Is it possible to do a fresh review, just in case for things to be okay?

Thanks again!