search

christophergandrud / imfr

R package for interacting with the IMF RESTful JSON API
49 stars 5 forks source link

Major update to imfr Version 2, including backward compatibility and unit testing #40

Closed chriscarrollsmith closed 1 year ago

chriscarrollsmith commented 1 year ago

Christopher,

Well, this was an exhausting task!

As we discussed, I've added backward compatibility with most of the original imfr data functions. These functions can still be called, and I've even improved their functionality somewhat. I also added comprehensive unit testing and fixed several bugs I encountered as I was building the tests.

I did not retain your original metadata functions. I think imf_metadata was useful and should be added back in a future version, but it needs to be fixed and the way it functions needs to be modified. (The way the inputs were set up didn't work for every IMF database, and I further broke it by removing the countries dataset. Shouldn't be terribly hard to fix, but not sure I have the bandwidth for it right away.) I hope you'll agree its functionality was not "core" enough to stand in the way of deploying of the rest of the package.

Since I can't push to the repo, I can't create a pkgdown website. Assuming you accept the pull request, you may want to consider doing so. (I've never done it before, but I understand you can use pkgdown::build_site_github_pages.) I also can't submit to CRAN (unless I have a written statement from you authorizing me to do so), so I suggest doing that as well.

I've been coding in R for a couple years, but this is my very first pull request, so it feels like a big deal! I tried to cover all my bases, but please let me know if there's anything I've done wrong.

Thanks,

Chris

chriscarrollsmith commented 1 year ago

On second thought, hold off on this request. I think there's one minor backward-compatibility issue I've yet to resolve.