search

sboysel / fredr

An R client for the Federal Reserve Economic Data (FRED) API
https://sboysel.github.io/fredr/
Other
92 stars 21 forks source link

CRAN issues #75

Closed sboysel closed 3 years ago

sboysel commented 4 years ago

Package was dropped from CRAN today due to some outstanding issues. The CRAN check results are no longer available either. travis build have been failing so it could be related. Will look into it this week to get it accepted again.

AdeelK93 commented 4 years ago

Is the CRAN rejection due to the tests failing? I could be totally off, but from looking at it, it looks like test_docs.R lines 8 and 9 are failing because GETting a bad documentation endpoint like https://fred.stlouisfed.org/docs/api/fred/foo.html would once give a non-200 status code, but now yields a 200.

Big fan of your work! I use fredr all the time

sboysel commented 4 years ago

Thanks @AdeelK93! Seems strange bad endpoints would be tolerated like that. Since there are relatively few, I suppose we could hardcode them and forgo status code checking.

sboysel commented 4 years ago

@AdeelK93 @DavisVaughan I'm considering changing fredr_docs() to just fredr_docs() <- function() utils::browseURL(url = "https://api.stlouisfed.org/docs/fred/"). I feel like this is easily the least practical function in the package and this simplification would be more user and maintainer friendly.

DavisVaughan commented 4 years ago

That’s fine by me - is that the only failure?

sboysel commented 4 years ago

Yes. Other than the entire test suite taking over 20 minutes for all the endpoints.

I'll try messing around with nealrichardson/httptest to see if we can cache requests in the package. If I'm understanding the documentation correctly, this amounts to

  1. Running the requests wrapped in capture_requests() once (caches responses in tests/testthat/)
  2. Wrapping all calls to test_that() in the test suite with with_mock_api()
DavisVaughan commented 4 years ago

I think that sounds pretty nice. The biggest thing for me would be "sometimes I actually want to run the tests against the live API. how do i do that?" The answer seems to be: https://github.com/nealrichardson/httptest#how-do-i-switch-between-mocking-and-real-requests

So what might be nice is to have it set up so that 99% of the time we test against the mocked/cached requests, but we then have an env var that we can set to test against the real live FRED api. CRAN would always run against the mocked tests, but we could have a Github Actions build that runs on a cron job (maybe 1 time a week?) that would run against the live API by setting the env var. That would notify us if anything fails, without putting the package in danger of being removed from CRAN. We would also have the standard set of Github Action builds that run against the mocked requests

sboysel commented 4 years ago

Ok sounds good. Github Actions is less familiar to me. Is this the right documentation? https://help.github.com/en/actions/reference/events-that-trigger-workflows

DavisVaughan commented 4 years ago

Maybe start here https://github.com/r-lib/actions/blob/master/README.md

And usethis::use_tidy_github_actions()

I can have a look next week to set things up if that would help

sboysel commented 4 years ago

Discussion of mock API can be moved to #73. Just added some notes on some issues I'm facing to that issue.

sboysel commented 4 years ago

Change to fredr_docs() was merged in #77. Will close this issue when fredr is back on CRAN (i.e. must address #73 before resubmitting).

mhollander commented 4 years ago

Is there an update on getting fredr back on CRAN?

sboysel commented 4 years ago

@mhollander We've been trying to overhaul/improve the testing suite to be more CRAN friendly in #79 but, consistent with most of my development experience, introduced more issues along the way. In a nutshell, the full testing suite consists of many calls to the FRED API and have been timing out and/or failing since introducing the mocking framework. Unfortunately, I haven't had as much time to work on FOSS stuff recently but I will revisit when I can and then resubmit to CRAN. Any PRs to that end are welcome and appreciated.

mhollander commented 4 years ago

Thank you for the comment. I completely understand that you don't have time for FOSS recently. Getting the tests to work is beyond my knowledge (and I don't have much FOSS time myself!). For now, I'm just installing from github. I hope one day to be able to help out.

thsavage commented 4 years ago

First, let me thank for developing this API. I have used it extensively for teaching purposes. And I know finance practitioners have found it very valuable. The deprecated version now is no longer useable with R 4.0.2 (and I'm surprised by how many changes have been pushed out by CRAN in the summer of 2020). Any chance that you will do another build?

AdeelK93 commented 4 years ago

I know it's sloppy, but would you be open to cutting out some of the tests in order to get this back on CRAN, if issues with the testing suite are still holding this project back?

sboysel commented 4 years ago

@thsavage @AdeelK93 @mhollander thanks for your interest in the package! Yes, I would be open to reducing some of the testing suite for the sake of getting fredr back on CRAN. I can make the changes in master to resubmit to CRAN and continue to iron out the testing issues in a development branch. I will be able to put some more time to this in the coming weeks. Appreciate everyone's patience.

alexWhitworth commented 4 years ago

My $0.02 is there's no need to rush getting it back on CRAN, since it's easy enough to install from either source or GitHub.

@sboysel -- are you able to provide an overview of the test suite problems? I'd be happy to submit a PR if given some general direction to get started. But I'm lazy and I don't want to wade through the code w/o such an overview :)

thsavage commented 4 years ago

@sboysel: From one economist to another, again my gratitude for what you have done. (I remember the busy days of grad school, which is your first commitment.)

The ability to ingest data in the first class of a semester allows me to teach an immersion class rather than the traditional econometrics class. The ability to do time series storytelling really empowers the masters student.

sboysel commented 4 years ago

Thanks to everyone for their interest in and support of fredr. Even more thanks for everyone's patience as we have worked to get fredr and its cumbersome testing suite back on CRAN since it was dropped in April.

Wanted to update everyone on the amazing efforts of @DavisVaughan in preparing fredr for a 2.0.0 release and getting back on CRAN. All credit should go to Davis for keeping fredr up to date with the CRAN and GitHub packaging ecosystems. Davis's work can be previewed in the release candidate #84 .

We are releasing the package shortly and a summary of changes can be viewed here. All modifications are internal and there should be no breaking changes. I will update everyone on this issue once fredr is officially back on CRAN. Let us know if you have any feedback or run into any issues.

mhollander commented 4 years ago

This is awesome! Thanks!

On Thu, Nov 5, 2020 at 11:38 AM Sam Boysel notifications@github.com wrote:

Thanks to everyone for their interest in and support of fredr. Even more thanks for everyone's patience as we have worked to get fredr and its cumbersome testing suite back on CRAN since it was dropped in April.

Wanted to update everyone on the amazing efforts of @DavisVaughan https://github.com/DavisVaughan in preparing fredr for a 2.0.0 release and getting back on CRAN. All credit should go to Davis for keeping fredr up to date with the CRAN and GitHub packaging ecosystems. Davis's work can be previewed in the release candidate #84 https://github.com/sboysel/fredr/pull/84 .

We are releasing the package shortly and a summary of changes can be viewed here https://drive.google.com/file/d/1qAcgxyTXM3RaSgLmTU9eBWPqh_eSAML5/view?usp=sharing. All modifications are internal and there should be no breaking changes. I will update everyone on this issue once fredr is officially back on CRAN. Let us know if you have any feedback or run into any issues.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/sboysel/fredr/issues/75#issuecomment-722493139, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAMI4JB7S34G4GNLF2IOSRDSOLIJNANCNFSM4MNOQFXA .

thsavage commented 4 years ago

Indeed. Many thanks.

Fredo-XVII commented 4 years ago

I've been following this through GitHub, awesome job!

Congrats on getting done!

On Thu, Nov 5, 2020, 11:18 AM Timothy H. Savage notifications@github.com wrote:

Indeed. Many thanks.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/sboysel/fredr/issues/75#issuecomment-722517273, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACDESHGRSLLH6K5LBA2AEITSOLM5PANCNFSM4MNOQFXA .

yanlesin commented 4 years ago

Thanks a lot for getting this back on CRAN! I was following it as well, great learning experience! Thanks @DavisVaughan and @sboysel!

sboysel commented 3 years ago

fredr is now back on CRAN! Will be merging #84 into master shortly and drafting a GitHub release of 2.0.0. Thanks for bearing with us!

thsavage commented 3 years ago

Great work. Thank you very much.