UBC-MDS / software-review-2022

0 stars 0 forks source link

Submission Group 11: cryptocurrencyeda(Python) #14

Open nobbynguyen opened 2 years ago

nobbynguyen commented 2 years ago

Submitting Authors:

Package Name: cryptocurrencyeda One-Line Description of Package: A Python package for simple exploratory data analysis of historical cryptocurrency prices and performance. Repository Link: cryptocurrencyeda Version submitted: 1.0.1 Editor: Florencia D'Andrea (@flor14)

Reviewers:


Description

This is a Python package to analyze historical cryptocurrency prices and performance through simple exploratory data analysis including calculations and plotting. Data is sourced from the KuCoin API. There are four functions that are included in this python package which are described in more detail below. Cryptocurrency investors and enthusiasts can use this package to analyze cryptocurrencies of interest. There are four functions in our packages:

JOSS Checks - [ ] The package has an **obvious research application** according to JOSS's definition in their [submission requirements][JossSubmissionRequirements]. Be aware that completing the pyOpenSci review process **does not** guarantee acceptance to JOSS. Be sure to read their submission requirements (linked above) if you are interested in submitting to JOSS. - [ ] The package is not a "minor utility" as defined by JOSS's [submission requirements][JossSubmissionRequirements]: "Minor ‘utility’ packages, including ‘thin’ API clients, are not acceptable." pyOpenSci welcomes these packages under "Data Retrieval", but JOSS has slightly different criteria. - [ ] The package contains a `paper.md` matching [JOSS's requirements][JossPaperRequirements] with a high-level description in the package root or in `inst/`. - [ ] The package is deposited in a long-term repository with the DOI: *Note: Do not submit your package separately to JOSS*

Are you OK with Reviewers Submitting Issues and/or pull requests to your Repo Directly?

This option will allow reviewers to open smaller issues that can then be linked to PR's rather than submitting a more dense text based review. It will also allow you to demonstrate addressing the issue via PR links.

michelle-wms 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:

Readme requirements The package meets the readme requirements below:

The README should include, from top to bottom:

Usability

Reviewers are encouraged to submit suggestions (or pull requests) that will improve the usability of the package as a whole. Package structure should follow general community best-practices. In general please consider:

Functionality

For packages co-submitting to JOSS

Note: Be sure to check this carefully, as JOSS's submission requirements and scope differ from pyOpenSci's in terms of what types of packages are accepted.

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

Final approval (post-review)

Estimated hours spent reviewing: 1 hour


Review Comments

What an interesting package this is, especially on the wave of crypto hype this could prove to be quite a popular package in time to come! Good job with creating this innovative package! Some thoughts for further improvements:

dol23asuka 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:

Readme requirements The package meets the readme requirements below:

The README should include, from top to bottom:

Usability

Reviewers are encouraged to submit suggestions (or pull requests) that will improve the usability of the package as a whole. Package structure should follow general community best-practices. In general please consider:

Functionality

For packages co-submitting to JOSS

Note: Be sure to check this carefully, as JOSS's submission requirements and scope differ from pyOpenSci's in terms of what types of packages are accepted.

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

Final approval (post-review)

Estimated hours spent reviewing: 1 hour


Review Comments

  1. In the README.md under the Import all functions: section, it would be better to remove the <<< for easier copy as a whole. I first copied the whole chunk into Python and it will throw message like SyntaxError: invalid syntax. Should be an easy update!

  2. For function retrieve_data(symbol="BTC-USDT", time_period="1day", start_date="2018-01-01", end_date="2022-01-10", ), could be better to explain a little bit what symbol is. I am still a little bit confused about it even after reading the documents.

  3. The x-axis of the plot output does not show a clear time range like from which year to which year as there is only a monthly interval. Could also include year information in the x-axis of the plot for easier understanding.

  4. In your example usage website link(https://cryptocurrencyeda.readthedocs.io/en/latest/example.html#imports), there is ab error message ModuleNotFoundError: No module named 'cryptocurrencyeda' under the Import section. Should be an easy fix.

  5. You can also plot the daily growth rate for users (can be a new function in the future). Overall, it is a good function. Nice job! By the way, you did not included the codecov badge in README. Could have a simple update!

nd265 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:

Readme requirements The package meets the readme requirements below:

The README should include, from top to bottom:

Usability

Reviewers are encouraged to submit suggestions (or pull requests) that will improve the usability of the package as a whole. Package structure should follow general community best-practices. In general please consider:

Functionality

For packages co-submitting to JOSS

Note: Be sure to check this carefully, as JOSS's submission requirements and scope differ from pyOpenSci's in terms of what types of packages are accepted.

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

Final approval (post-review)

Estimated hours spent reviewing: 45 minutes


Review Comments

  1. A very interesting package, with great ease of use for all types of users. Great work team !!
  2. In ReadTheDocs, there is a ModuleNotFoundError under the Imports section, which should be fixed as it is the official documentation
  3. It would be better if you could include the badges for CodeCov and docs(for showing that the docs are rendered properly whenever a change is pushed to main branch as part of release)
  4. It would be helpful if >>> can be removed from the README file under Import all functions and use the functions so that it is easy for the user to copy and directly try out your code
  5. It would be good if the value returned by avg_daily_return can be rounded off to nearest 2 or 3 decimal places for better readability and interpretability.
  6. In API Reference section in ReadTheDocs, daily_growth_rate function, the parameters are not being rendered as bullet points like in other functions image
  7. In function plot_price, the Examples heading is missing under which >>>plot_price(df) should be written image

Both 6 and 7 points above can be rectified by tweaking your python function docstrings

  1. A future implementation for daily_growth function could be to plot the graph for easy visualisation for better interpretability