Comments (24)
@sboysel I think this is ready for a CRAN release. I've got a release candidate PR ready to go #84.
If you approve, then I can release it with devtools::release()
and then you just have to accept the email (since you are the Maintainer).
Overall changes:
GitHub Actions
Moved away from Travis and Appveyor to GitHub Actions. The "workflows" are in .github/workflows/
-
R-CMD-check.yaml
runscheck()
across many versions of R and many OSs (Mac, Windows, Linux). However it does not run with an API key. This mocks how we will run tests and examples on CRAN. -
R-CMD-check-auth.yaml
runscheck()
on the latest version of R on a Mac and does run with the API key. This build runs on all pushes to master and all PRs. The total check time is around 4 minutes, so it seems reasonable to do this on 1 build at all times. -
test-coverage.yaml
runscovr()
on the package. It always runs with an API key to get accurate coverage.-
It runs on a cron job every monday at 1am UTC
-
It runs whenever you push to master and have
[covr]
in the commit message
-
-
pkgdown.yaml
builds the pkgdown site and automatically pushes it to the gh-pages branch. It always runs with an API key to have nice example output.-
It runs on a cron job every monday at 12am UTC
-
It runs whenever you push to master and have
[pkgdown]
in the commit message
-
Examples
Examples have been changed from donttest{}
to using the new fredr_has_key()
function. This makes the examples run whenever a key is available (like with pkgdown), but otherwise they are skipped (like on CRAN). This is what googlesheets4 does. Eventually roxygen2 will support this more cleanly with the @examplesIf
tag, but this PR hasn't been merged yet r-lib/roxygen2#993
Tests / Check
All tests that depend on a key use skip_if_no_key()
to avoid failures on CRAN. Our github checks that run without an API key keep us honest and ensure that everything will work when we submit to CRAN without a key.
The entire check takes around 30 seconds without a key, and 4 minutes with a key.
Each R/<name>.R
file now has a corresponding tests/testthat/test-<name>.R
file. This makes it easier to follow as devtools::test()
runs. It has the side benefit of making usethis::use_test()
jump directly to the corresponding test file when you call it without any args when you have the R/<name>.R
file open.
Changes
-
New
fredr_has_key()
andfredr_get_key()
which are mainly just utility functions -
Refreshed all vignettes, the readme, and many function docs
-
A number of FRED URLs had to be updated since they redirect. It is a new-ish thing in R's check process where they don't want URLs to redirect. I just replaced them with what they redirect to.
-
All functions can now retry a request if we hit a rate limit. See my previous github comment for more information.
-
Overhauled the infrastructure behind
capture_args()
to be a bit safer. We now validate the type/size of every arg that comes through. -
I removed the default of
NULL
for required arguments. Generally the best practice is to leave these without an argument, and only give defaults to optional arguments. This especially makes it less confusing forfredr_category_related_tags()
where bothcategory_id
andtag_names
are required. -
I have also placed
...
between the required arguments and the optional arguments in a function signature. Internally I check that these...
are empty. Placing the...
makes it so that optional arguments have to be specified by name, resulting in more self documenting code. Checking that...
are empty prevents the user from making a typo and having it get swallowed silently. This is something that we are using more and more in the tidyverse to help users write better code. It also means we can add extra optional arguments whenever we want without fear of breaking any code (since they had to be specified by name).
from fredr.
Thanks @DavisVaughan. Will do, good suggestion on the email followup. Will update you when I hear back.
from fredr.
Just received word that the latest submission of fredr
is heading to CRAN!
from fredr.
Thanks for all the help, @DavisVaughan!
from fredr.
Good to hear from you @DavisVaughan. Was just clearing the cobwebs from my R development environment before reading this issue. I think this all sounds great and I'm happy to follow the advice of someone more familiar with this whole process than I. I will give you push permissions to master and I sincerely appreciate your help. I can handle any additional changes and the CRAN resubmission process. Let me know what else I can do.
from fredr.
I've added you as a collaborator to the repository and I don't seem to have any special branch restrictions set up. Let me know if that's not enough access.
from fredr.
Thanks! Happy to help. I'll also need you to go into the Settings part of the repo and add a fredr token to the Secrets section so it can be used in github actions (I dont have access to Settings, but I think that is expected). You would create a Secret with the name FRED_API_KEY
and then you'd add your key, and then we can set it as an environment variable in github actions with env: FRED_API_KEY = {secrets.FRED_API_KEY}
or something similar to that
from fredr.
Done. FRED_API_KEY
was already set so I just verified it's the same test key we've always used.
from fredr.
@sboysel could you please go into Settings -> scroll down to GitHub Pages -> change Source to the gh-pages branch (the folder should just be root, I think that is the default)
from fredr.
Could you also please turn off the AppVeyor Webhook? I think you would need to go into Setting -> Webhooks -> Delete anything that looks related to AppVeyor (or travis if you see it)
from fredr.
@sboysel could you please go into Settings -> scroll down to GitHub Pages -> change Source to the gh-pages branch (the folder should just be root, I think that is the default)
Done.
from fredr.
Could you also please turn off the AppVeyor Webhook? I think you would need to go into Setting -> Webhooks -> Delete anything that looks related to AppVeyor (or travis if you see it)
Done. Should we keep the Codecov webhook?
from fredr.
Should we keep the Codecov webhook?
Yes!
Thanks!
from fredr.
It seems I've hit the rate limit for the stored api key. I'm asking the people at FRED if they can up our limits (they specifically say that you can ask them to raise it here https://fred.stlouisfed.org/docs/api/fred/errors.html)
Once the dust settles, the cron jobs that run weekly shouldn't exceed that limit, but in the meantime its annoying.
from fredr.
I've learned that the rate limit is 120 requests per minute. This is pretty generous, and is hard to hit unless you are either:
- Constantly requesting short series from a fast responding endpoint
- Running multiple requests in parallel using the same api key
The second is what happens when I run the test coverage build. It runs test coverage with the api key + the R CMD Check build that uses the api key.
Now that we know what the rate limit is, I've added in some handling for it. We now sleep for 20 seconds when we get rate limited (the http code is 429 so we can detect this), then try again. We try for a max of 6 times which should be enough to unlimit ourselves (2 full minutes is bound to be enough time...)
I can test that it works pretty well with this, which does rate limit me at around request 120
for (i in 1:500) {
fredr("UNRATE", observation_start = Sys.Date() - 50L)
message("request ", i)
}
from fredr.
This is amazing work, @DavisVaughan ! A HUGE thanks for helping out and doing a much better job than if I handled it alone. Great to learn all these tricks for R packaging.
I've looked over your changes and it all looks great to me. I'll gladly defer to your expertise on areas I'm less familiar with.
Yes, please do submit via devtools::release()
and I will monitor my email. I will make announcements on the remaining issues (#75) and Twitter. I will also close #82 once the package has been accepted.
from fredr.
I'm happy to help!
I have released fredr 2.0.0, you should have received an email.
I will also close #82 once the package has been accepted.
Just let me know when you hear back from CRAN (acceptance or otherwise). If accepted, I'll merge in #84 and that will take care of closing all the linked issues. Then I'll update the version number again to bump it to the next development version and that should be it.
from fredr.
Hmm, I don't see fredr in the CRAN ftp incoming site anymore, but it isn't on CRAN. Did we get rejected?
from fredr.
Just got this email from CRAN last night:
Thanks,
Please always make sure to reset to user's options(), working directory
or par() after you changed it in examples and vignettes and demos.
e.g.:old <- options(digits = 3)
...
options(old)Additionally:
Have the issues why your package was archived been fixed?
Please explain this in the submission comments.Please fix and resubmit.
Best,
Gregor Seyer
- Seems I was using
options(digits = 4)
in each of the vignettes (just checked and does not appear anywhere else) - I didn't get to see the actual errors CRAN was citing before the package was dropped. Naturally, the web check results page they directed me to is no longer active. Should it just suffice to say the summary of changes in 2.0.0 address issues with our testing suite that led our package to be dropped? Sorry I cannot provide more useful details.
from fredr.
@sboysel - I've resubmit fredr to CRAN
Can you please check your email for another CRAN submission email and accept the submission?
Additionally, can you please respond to Gregor with something like:
Gregor,
Thanks for the review! We have resubmit fredr.
- We have removed the calls to `options()` in the vignettes.
- We added comments about why the package was archived, and how we fixed it. We were hitting a URL that is no longer valid, and this broke a test.
Thanks,
Sam
I find that when I reply nicely saying that we resubmit and fixed their issues, I get a faster turnaround time.
from fredr.
Great! I see it on cran! https://cran.r-project.org/web/packages/fredr/
from fredr.
Alright. All merged and cleaned up!
from fredr.
On the home page of the github repo, you link to the pkgdown site with HTTP, you may want to switch to HTTPS! (Both work, you should just have to change the url on the repo page)
from fredr.
Will do. Also making a note here to change the Shields.io badges in README.md
/ README.Rmd
so that the proper CRAN version appears.
from fredr.
Related Issues (20)
- Unable to Install (Broken Dependency?) HOT 2
- Support for FRED Maps HOT 3
- Option to select Seasonally Adjusted Observation HOT 1
- upper bound too low for limit in fredr_category_series() HOT 3
- Pagination HOT 1
- Encoding error with fredr_series_observations HOT 11
- Better test environment for API requests HOT 3
- CRAN issues HOT 25
- Removed from CRAN? HOT 2
- Fredr can't return a legit series_id HOT 3
- Release fredr 2.0.0
- way to traverse category trees? HOT 4
- This likely isn't an issue with your code, but something to be wary of HOT 3
- Trouble downloading multiple FRED series HOT 5
- fredr() and parameter vintage_dates HOT 5
- M2 not FedFunds
- Release fredr 2.1.0 HOT 1
- fredr appears to be completely broken on my R setup HOT 6
- Capture multiple series simultaneously? HOT 5
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from fredr.