-
-
Notifications
You must be signed in to change notification settings - Fork 104
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
yfR: Downloads and Organizes Financial Data from Yahoo Finance #523
Comments
Thanks for submitting to rOpenSci, our editors and @ropensci-review-bot will reply soon. Type |
🚀 The following problem was found in your submission template:
👋 |
Oops, something went wrong with our automatic package checks. Our developers have been notified and package checks will appear here as soon as we've resolved the issue. Sorry for any inconvenience. |
Checks for yfR (v0.0.1)git hash: c345549c
Important: All failing checks above must be addressed prior to proceeding Package License: MIT + file LICENSE 1. Statistical PropertiesThis package features some noteworthy statistical properties which may need to be clarified by a handling editor prior to progressing. Details of statistical properties (click to open)
The package has:
Statistical properties of package structure as distributional percentiles in relation to all current CRAN packages
All parameters are explained as tooltips in the locally-rendered HTML version of this report generated by the The final measure (
1a. Network visualisationClick to see the interactive network visualisation of calls between objects in package 2.
|
function | cyclocomplexity |
---|---|
yf_get | 23 |
yf_get_single_ticker | 22 |
Static code analyses with lintr
lintr found the following 2 potential issues:
message | number of times |
---|---|
Avoid library() and require() calls in packages | 2 |
Package Versions
package | version |
---|---|
pkgstats | 0.0.3.96 |
pkgcheck | 0.0.2.276 |
Editor-in-Chief Instructions:
Processing may not proceed until the items marked with ✖️ have been resolved.
@jooolia The faling check is just because the README does not have a CI badge. @msperlin Could you please add an R CMD check badge to your readme? (We check for CI via badges rather than workflow results, because we do accept submissions from arbitrary code-hosting platforms, not just GitHub.) Thanks! |
Good morning. Sure, I just added the R-CMD badge. |
@ropensci-review-bot check package |
Thanks, about to send the query. |
🚀 Editor check started 👋 |
Oops, something went wrong with our automatic package checks. Our developers have been notified and package checks will appear here as soon as we've resolved the issue. Sorry for any inconvenience. |
Checks for yfR (v0.0.1)git hash: 1ee2f6f5
Package License: MIT + file LICENSE 1. Package DependenciesDetails of Package Dependency Usage (click to open)
The table below tallies all function calls to all packages ('ncalls'), both internal (r-base + recommended, along with the package itself), and external (imported and suggested packages). 'NA' values indicate packages to which no identified calls to R functions could be found. Note that these results are generated by an automated code-tagging system which may not be entirely accurate.
Click below for tallies of functions used in each package. Locations of each call within this package may be generated locally by running 's <- pkgstats::pkgstats(<path/to/repo>)', and examining the 'external_calls' table. basec (6), file.path (5), as.Date (4), min (4), paste0 (4), data.frame (3), file.exists (3), length (3), list (3), seq (3), as.character (2), as.numeric (2), for (2), max (2), names (2), options (2), rep (2), switch (2), tempdir (2), as.POSIXct (1), class (1), file (1), is.na (1), lapply (1), list.files (1), order (1), seq_along (1), setdiff (1), sum (1), Sys.Date (1), Sys.getenv (1), which (1) yfRfix_ticker_name (2), get_morale_boost (2), set_cli_msg (2), yf_get_available_indices (2), calc_ret (1), date_to_unix (1), fct_format_wide (1), unix_to_date (1), yf_get (1), yf_get_available_collections (1), yf_get_ibov_stocks (1), yf_get_index_comp (1), yf_get_single_ticker (1) dplyrfirst (3), bind_rows (2), tibble (2), filter (1), lag (1), mutate (1), rename (1) purrrmap (2), map_chr (2), pmap (1) readrread_rds (4), write_rds (1) stringrfixed (1), str_c (1), str_detect (1), str_split (1) rvesthtml_nodes (2), html_table (1) utilsdata (2), capture.output (1) furrrfurrr_options (2) futureavailableCores (1), plan (1) lubridatewday (2) tidyrall_of (1), pivot_wider (1) quantmodgetSymbols (1) tibbletibble (1) zooindex (1) 2. Statistical PropertiesThis package features some noteworthy statistical properties which may need to be clarified by a handling editor prior to progressing. Details of statistical properties (click to open)
The package has:
Statistical properties of package structure as distributional percentiles in relation to all current CRAN packages
All parameters are explained as tooltips in the locally-rendered HTML version of this report generated by the The final measure (
2a. Network visualisationClick to see the interactive network visualisation of calls between objects in package 3.
|
name | conclusion | sha | date |
---|---|---|---|
pages build and deployment | success | 1ee2f6 | 2022-03-31 |
pkgdown | success | 51af0f | 2022-03-30 |
R-CMD-check | success | 1ee2f6 | 2022-03-31 |
render-rmarkdown | failure | f3dbe5 | 2022-03-30 |
test-coverage | success | 1ee2f6 | 2022-03-31 |
3b. goodpractice
results
R CMD check
with rcmdcheck
rcmdcheck found no errors, warnings, or notes
Test coverage with covr
Package coverage: 87.78
Cyclocomplexity with cyclocomp
The following functions have cyclocomplexity >= 15:
function | cyclocomplexity |
---|---|
yf_get | 23 |
yf_get_single_ticker | 22 |
Static code analyses with lintr
lintr found the following 2 potential issues:
message | number of times |
---|---|
Avoid library() and require() calls in packages | 2 |
Package Versions
package | version |
---|---|
pkgstats | 0.0.4.4 |
pkgcheck | 0.0.3.6 |
Editor-in-Chief Instructions:
This package is in top shape and may be passed on to a handling editor
Dear @msperlin, |
Good morning Julia, The main goal of yfR is to help user download large ammounts of data from Yahoo Finance (YF). Packages quantmod and tidyquant also offers a function for downloading price data from YF, but only that. Besides importing data, yfR offers the following functionalities:
|
Thank you @msperlin, I am discussing with the other editors and will get back to you. Thanks, Julia |
Thanks for your patience @msperlin. The fit seems to be good for us and I am now looking for a handling editor. |
Great, thanks @jooolia. |
@ropensci-review-bot assign @melvidoni as editor |
Assigned! @melvidoni is now the editor |
@ropensci-review-bot seeking reviewers |
Please add this badge to the README of your package repository: [](https://github.com/ropensci/software-review/issues/523) Furthermore, if your package does not have a NEWS.md file yet, please create one to capture the changes made during the review process. See https://devguide.ropensci.org/releasing.html#news |
Thanks. The badge is added in dc712f4abac246604721ed7f2926f9794e4e7f99 and the news file already exists. |
Dear @thisisnic, please find my replies below. Again, thanks for the time reviewing my package. I've made many changes based on your comments. All changes are in the main branch. Review Comments
Thanks! I'm glad you enjoyed using it.
Yes, this is fixed in the latest commit 2749f6fbdad562fe6f963e409470916d58f6fdb2.
Comments on yf_get.R
Thanks.
Fixed.
Fixed.
You're right. I changed the code so that the user choice is always respected.
Done.
I agree. It is becoming a long code. I'll keep that in mind. Comments on collections.R
Thanks!
The idea is for later to have collections that are not indices. This is way I broke it into separate functions.
Done.
My mistake. Its fixed.
I followed the convention of using "object""verb" when naming functions. In fact, I just realized I used the wrong convention in some functions. Comments on yf_convert_to_wide.R
Done. Comments on yf_get_clean_data.R
Done. Comments on yf_get_single_ticker.R
Thanks. I really like it too. Comments on "getting started" vignette
Agreed. To fix it, I increased the time time period and removed the geom_point() layer. |
@thisisnic and @s3alfisc what are your thoughts on the changes? |
Hi @melvidoni, I will try to take a closer look at @msperlin's comments this weekend / early next week =) |
Thanks for the reminder there @melvidoni - I am happy with the changes made. |
Thanks @thisisnic! We will wait until after the weekend for @s3alfisc's comments then! |
Hi all, I am very happy with the changes. The package looks and works great! In particular, I am super happy about how the documentation has evolved! :) |
@ropensci-review-bot approve yfR |
Approved! Thanks @msperlin for submitting and @s3alfisc, @thisisnic for your reviews! 😁 To-dos:
Should you want to acknowledge your reviewers in your package DESCRIPTION, you can do so by making them Welcome aboard! We'd love to host a post about your package - either a short introduction to it with an example for a technical audience or a longer post with some narrative about its development or something you learned, and an example of its use for a broader readership. If you are interested, consult the blog guide, and tag @ropensci/blog-editors in your reply. She will get in touch about timing and can answer any questions. We maintain an online book with our best practice and tips, this chapter starts the 3d section that's about guidance for after onboarding (with advice on releases, package marketing, GitHub grooming); the guide also feature CRAN gotchas. Please tell us what could be improved. Last but not least, you can volunteer as a reviewer via filling a short form. |
Thanks @s3alfisc, @thisisnic! Realy appreciate the feedback. I'll make all changes and report here soon. |
@ropensci-review-bot finalize transfer of yfR |
Transfer completed. |
@s3alfisc, @thisisnic Can I include you both as reviewers in the DESCRIPTION file? |
Of course, @msperlin , thanks! |
@melvidoni I've made all required changes. Please let me know if anything else is needed. |
Same with me @msperlin, and thanks! :) |
Excellent, thanks. Please, tick all the boxes in the corresponding post if you can. |
@melvidoni |
No worries, then it's fine @msperlin |
I'm planning to submit to CRAN tomorrow. Is that Ok? (sorry but I did not see any guideline about when to submit to cran) |
It should be okay. |
Hi @msperlin, We'd love to have a blog post about yfR, thank you for offering! We publish two types of articles, blog posts and technotes. Let us know which type of article you'd like to write and when you think you might have it ready by, and we'll pencil in a publication date that will give us time for a quick review (the date can always change if things go faster or slower than expected). We have a guide for blog authors (https://blogguide.ropensci.org/) and when you're ready and have a pull request, just set me as the reviewer or ping me and I'll come by to give you some feedback. Let me know if you have any questions, thanks! |
Thanks @steffilazerte. I'll have a look on the material you posted. But, I will need some time, maybe a couple of weeks to write the post. I'll let you know. |
Perfectly reasonable 😁 |
@steffilazerte I wrote a blog post. Just send you the PR and set your as reviewer. |
Date accepted: 2022-06-21
Submitting Author Name: Marcelo Perlin
Due date for @s3alfisc: 2022-05-29Submitting Author Github Handle: @msperlin
Other Package Authors Github handles: (comma separated, delete if none)
Repository: https://github.com/msperlin/yfR
Version submitted: 0.0.1
Submission type: Standard
Editor: @melvidoni
Reviewers: @s3alfisc, @thisisnic
Due date for @thisisnic: 2022-06-13
Archive: TBD
Version accepted: TBD
Language: en
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.):
Explain how and why the package falls under these categories (briefly, 1-2 sentences):
Package yfR retrieves and organizes data from Yahoo Finance, a large repository for stock price data.
Target audience are students, researchers and industry practioneers in the field of Finance and Economics.
Package yfR is the second and backwards-incompatible version of BatchGetSymbols, also developed by me. My plan is to first deprecate BatchGetSymbols and later remove it from CRAN and archive it in Github.
Moreover, there are other packages, such as quantmod, that downloads data from Yahoo Finance, but none with similar features to yfR and BatchGetSymbols.
Yes.
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.
Explain reasons for any
pkgcheck
items which your package is unable to pass.Unfortinately, I was not able to run pkgcheck locally as I was unable to install (or make) dependency ctags in my Linux Mint 20.3 machine. Nonetheless, I read through and followed all guidelines available in the manual.
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
Code of conduct
The text was updated successfully, but these errors were encountered: