rOpenGov / pxweb

R tools to access PX-WEB API
http://ropengov.github.io/pxweb
Other
69 stars 31 forks source link

Feature request: export pxweb_parse_response() #249

Closed majazaloznik closed 1 year ago

majazaloznik commented 1 year ago

Hi, would it be possible to export the pxweb_parse_response() function please? Thanks! 😃

MansMeg commented 1 year ago

That would probably be possible. Can you give a usecase on how (and why) you would like to use such a function.

majazaloznik commented 1 year ago

oh, I just want to use it inside a (personal) package as it reduces the overhead for my particular workflow. but I understand that using ::: notation in packages is risky.. the alternative is of course that I just make a copy of it and reuse it like that, but just thought I'd ask first.

MansMeg commented 1 year ago

That's fair enough. I just wonder how this should be implemented in the best way. Maybe call the exported function something else, like as.pxweb(), pxweb_httr_response() or pxweb_parse_httr_response() ? I think I rather add a new function to the API with the same functionality so I can keep the other function out of the API. But it should be easy to add an external function with the same behaviour, so you can use it.

Does that seem good to you? Then you could wrap your function using pxweb:::pxweb_parse_response() for now, and then simply swap it?

majazaloznik commented 1 year ago

Not sure I completely understand the logic of duplicating it, but either way, that's completely your call, I'll be happy with whatever you call it 😄

MansMeg commented 1 year ago

Now fixed with PR #251. It was, as you said, easier to just export the function. i have now done that, but I wait with updating this on CRAN for now. Although, you can install it from github right away.

majazaloznik commented 1 year ago

You rock MÃ¥ns, thanks for sorting me out so quickly! :rocket:

MansMeg commented 1 year ago

No problem! Thanks for submitting an issue!