UBC-MDS / software-review-2023

DSCI 524
0 stars 0 forks source link

Group 18 - fxtrackerR (R) #21

Open markusnam opened 1 year ago

markusnam commented 1 year ago

name: fxtrackerR about: Currency conversion, target rate lookup and plotting of rate history and profit/loss percentage history.


Submitting Author Name: Sarah Abdelazim, Lennon Au-Yeung, Crystal Geng, Markus Nam Submitting Author Github Handle: !--author1-->@missarah96<!--end-author1--, !--author2-->@lennonay<!--end-author2--, !--author3-->@THF-d8<!--end-author3--, !--author4-->@markusnam<!--end-author4-- Repository: https://github.com/UBC-MDS/fxtrackerR Version submitted: v0.3.0 Submission type: Standard Editor: Sarah Abdelazim, Lennon Au-Yeung, Crystal Geng, Markus Nam Reviewers: Austin Shih, Andy Wang, Bruce Wu, Shirley Zhang

Archive: TBD Version accepted: TBD Language: en

Package: fxtrackerR
Title: fxtrackerR is an R version of fxtracker, originally written in Python
Version: 0.0.0.9000
Authors@R: 
    c(person("Sarah", "Abdelazim", , "sarah_s96@hotmail.com", role = c("aut", "cre")),
    person("Markus", "Nam", , "chnam@student.ubc.ca", role = c("aut")),
    person("Crystal", "Geng", , "shuijing23884576@gmail.com", role = c("aut")),
    person("Lennon", "Au-Yeung", , "lennon.auyeung@me.com", role = c("aut")))
Description: This package can allow users to perform currency conversion based on the latest available exchange rate, look up a target exchange rate from historical data as well as plotting exchange rate history and profit/loss percentage history by specifying a currency pair.
License: MIT + file LICENSE
Encoding: UTF-8
Roxygen: list(markdown = TRUE)
RoxygenNote: 7.2.3
Suggests: 
    testthat (>= 3.0.0),
    rlang,
    covr,
    knitr,
    rmarkdown
Config/testthat/edition: 3
Imports: 
    dplyr,
    ggplot2, 
    tidyquant
VignetteBuilder: knitrt

Scope

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](http://besjournals.onlinelibrary.wiley.com/hub/journal/10.1111/(ISSN)2041-210X/journal-resources/policy-on-publishing-code.html)) - (*Scope: Do consider MEE's [Aims and Scope](http://besjournals.onlinelibrary.wiley.com/hub/journal/10.1111/(ISSN)2041-210X/aims-and-scope/read-full-aims-and-scope.html) 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

tiger12055 commented 1 year 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:


Review Comments

  1. It would be helpful to include your email and a link to your GitHub account in the contribution file so that others can easily get in touch with you regarding your contributions
  2. It is recommended to organize the functions into separate files based on their primary functionality. This will make it easier for readers to understand the main purpose of each function and also aid in writing unit tests. Grouping visualization functions together and other functions separately can enhance the readability and maintainability of the code.
  3. The documentation of your package is excellent and user-friendly. I was able to run the package smoothly without encountering any difficulties. Your group has paid close attention to every detail, making the user experience seamless. Great job!
  4. 99% test coverage. Great job
shlrley commented 1 year ago

Reviewer: Shirley Zhang

Package Review

Please check off boxes as applicable, and elaborate in the 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 hours


Review Comments

Great work on the R package! Here are a few suggestions/comments I have:

  1. Cross-referencing my comments 5 and 6 for your Python package, I liked how the graphs for price_trend_viz here are outputted directly onto the screen or in a pop-up window (when running on the terminal). If this is something possible to do with the Python package, that would be great.

  2. Similarly to the Python package, it would be nice to include citation information to the Yahoo Finance foreign exchange data, as well as any other sources you may have used to create the functions.

  3. The exception catching inside of your R script is really well done!

  4. I noticed that the x-axis labels are slanted when creating graphs with price_trend_viz(), but they are horizontal for graphs created with pl_trend_viz(). Perhaps one format could be chosen for consistency (I would recommend horizontal)?

  5. Furthermore, it looks like the y-axis on graphs created with pl_trend_viz() are in the wrong scale - the y-axis title is "Percentage", but the scales look like proportions. Perhaps the scale could be changed, to have an output similar to pl_trend_viz() in the Python package.

Congratulations again on creating a well-designed package, everything ran smoothly from beginning to end and I'm looking forward to seeing how the package will be improved!

austin-shih commented 1 year 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: 20 minutes


Review Comments

BruceUBC commented 1 year 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 hour


Review Comments

  1. It is a complete and perfect R package with well-documented functions and a clear user guide description. After following the process of installing your package and testing some of the functions, I can fully operate the functions of the package as a beginner.
  2. A possible improvement could be including a clickable link in the README part for reference. It would always be helpful and make your package trustworthy.
  3. Including plots in the usage part is good to explain the output of your functions. However, if the users are not familiar with the financial market and some related stuff. Some simple descriptions would be helpful for them to get to know the meaning of the plot.
  4. As I said in the Python package, some of the comments of the functions can be more detailed, making users easily understand the nature of the function.
  5. You can include contact information on the main page of the package besides writing in the CODE OF CONDUCT document. This will lead to convenience for any suggestions and advice from other users.
    It is a pleasure to read your package, and I quite learn a lot for constructing functions and tests for plots. Great work!