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

Group 18 - fxtrackerR (R) #21

Open
11 of 29 tasks
markusnam opened this issue Jan 31, 2023 · 4 comments
Open
11 of 29 tasks

Group 18 - fxtrackerR (R) #21

markusnam opened this issue Jan 31, 2023 · 4 comments

Comments

@markusnam
Copy link

markusnam commented Jan 31, 2023


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: @missarah96, @lennonay, @THF-d8, @markusnam
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

  • Paste the full DESCRIPTION file inside a code block below:
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", , "[email protected]", role = c("aut", "cre")),
    person("Markus", "Nam", , "[email protected]", role = c("aut")),
    person("Crystal", "Geng", , "[email protected]", role = c("aut")),
    person("Lennon", "Au-Yeung", , "[email protected]", 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

  • 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
    • data munging
    • data deposition
    • data validation and testing
    • workflow automation
    • version control
    • citation management and bibliometrics
    • scientific software wrappers
    • field and lab reproducibility tools
    • database software bindings
    • geospatial data
    • text analysis
  • Explain how and why the package falls under these categories (briefly, 1-2 sentences):
    This package retrieves data from Yahoo Finance to perform calculation, data lookup and plot generation.

  • Who is the target audience and what are scientific applications of this package?
    The target audience is anyone who is interested in the foreign exchange market. e.g. travellers, professional investors, financial institutions.

  • Are there other R packages that accomplish the same thing? If so, how does yours differ or meet our criteria for best-in-category?
    Some of the functions of an R package (priceR) are relevant to foreign exchange. But that is mainly for data retrieval. It does not provide visualizations and lookup function like fxtrackerR does. fxtrackerR allows user to visualize the trends and understand if a target price of a currency pair of interest is within a reasonable range.
    Another R package (czechrates) is also relevant to foreign exchange but it is only limited to Koruna to other currencies and no visualization as well.

  • (If applicable) Does your package comply with our guidance around Ethics, Data Privacy and Human Subjects Research?
    N/A

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

  • Explain reasons for any pkgcheck items which your package is unable to pass.
    N/A

Technical checks

Confirm each of the following by checking the box.

This package:

Publication options

  • Do you intend for this package to go on CRAN?

  • Do you intend for this package to go on Bioconductor?

  • Do you wish to submit an Applications Article about your package to Methods in Ecology and Evolution? If so:

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

@tiger12055
Copy link

tiger12055 commented Feb 3, 2023

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

  • Briefly describe any working relationship you have (had) with the package authors.
  • 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
  • Examples: (that run successfully locally) for all exported functions
  • 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.

Estimated hours spent reviewing:

  • 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

  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

@markusnam markusnam changed the title Group 18 - fxtrackerR (R) [placeholder] Group 18 - fxtrackerR (R) Feb 4, 2023
@shlrley
Copy link

shlrley commented Feb 7, 2023

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

  • Briefly describe any working relationship you have (had) with the package authors.
  • 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 the package and any non-standard dependencies in README
  • Vignette(s): demonstrating major functionality that runs successfully locally
  • Function Documentation: for all exported functions
  • Examples: (that run successfully locally) for all exported functions
  • 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.

Estimated hours spent reviewing: 1.5 hours

  • 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

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
Copy link

austin-shih commented Feb 8, 2023

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

  • Briefly describe any working relationship you have (had) with the package authors.
  • 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
  • Examples: (that run successfully locally) for all exported functions
  • 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.

Estimated hours spent reviewing: 20 minutes

  • 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

  • Fun function to play around with, can definitely see myself using it in the future!
  • I think I like it more to have the plots displayed in the same browser as opposed to the Python package.
  • Good job!

@BruceUBC
Copy link

BruceUBC commented Feb 9, 2023

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

  • Briefly describe any working relationship you have (had) with the package authors.
  • 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
  • Examples: (that run successfully locally) for all exported functions
  • 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.

Estimated hours spent reviewing: 1 hour

  • 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

  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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants