search

worldbank / pipr

R client to the PIP API
https://worldbank.github.io/pipr
Other
13 stars 4 forks source link

Circular logic in `display_aux` #48

Closed tonyfujs closed 1 year ago

tonyfujs commented 1 year ago

Hi @randrescastaneda

While reviewing Ronak's PR, I noticed something I had not caught while reviewing the PR that implemented display_aux

display_aux is supposed to be a wrapper around get_aux, but I just realized that get_aux is also calling display_aux. Looks like a bit of a circular logic, no? Why do we need get_aux to call display_aux? It should not be needed...

randrescastaneda commented 1 year ago

Hi @tonyfujs , Good question.

The reason is that, if you don't provide any value to get_aux you get a list of available auxiliary tables rather than an error... Again, this is to mimic Stata's behavior. When you don't provide inputs, it gives you something back for you to work with. See for example datalibweb. If you don't provide a country(), you will prompted with a list to select a region and then you can select a country. See images below. image

image

tonyfujs commented 1 year ago

Thanks @randrescastaneda But my understanding was that this was the role of display_aux, and that get_aux would keep behaving as before. Why do we need get_aux to behave like display_aux if display_aux already exists?

randrescastaneda commented 1 year ago

I totally understand your concern. And it is the role of display_aux. You're totally right. This is just, I thought, an additional functionality that we can provide to the user. get_aux works as usual for anything else. But, if you still think it is unnecessary, I can revert that and provide a message to the user, indicating that display_aux provides a list of available data. Is that ok?

randrescastaneda commented 1 year ago

This was closed in #50