search

lgnbhl / BFS

🇨🇭Search and Download Data from the Swiss Federal Statistical Office
https://lgnbhl.github.io/BFS
GNU General Public License v3.0
19 stars 5 forks source link

Simplify #4

Closed zambujo closed 3 years ago

zambujo commented 3 years ago

Hi Félix

I really liked your package but also had an issue with #3

As you marked help wanted, I thought I could have a look.

I think the issue had to do the the use of file.path() which already adds a path separator according to the platform linux/windows.

I found main.R a bit difficult to read, so I refactored a bit and in doing so I confirmed that value translation is still buggy and therefore I removed it for the time being.

I will still try to have a go at value translation on a separate pull request, but I thought you could find the following changes to be useful in the meantime.

I would be curious to see whether the proposed changes work on windows.

Thanks!

lgnbhl commented 3 years ago

Hi @zambujo,

Thank you so much for sharing and improving the existing code!

I just saw your pull request today. I've been very busy these days and I totally missed the notification of your pull request. My apologies!

It looks like your code is working well on my Windows. I am curious to know if it is possible to add back the translation in other languages. I will also see if there is a way to add it back based on your code improvements.

Best, Felix

lgnbhl commented 3 years ago

By the way @zambujo would you be interested in being added as a contributor or author of the package given your very valuable contribution?

lgnbhl commented 1 year ago

@zambujo I just discovered I didn't see you thumb up at my last question two years ago... So I just added your name as a contributor for the next BFS version release on CRAN https://github.com/lgnbhl/BFS/commit/d2c62e63f9c0fe9f78b74015021621ad40d97a45