Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Potential improvement on cache dir #223

Closed
dieghernan opened this issue Aug 6, 2021 · 9 comments · Fixed by #228
Closed

Potential improvement on cache dir #223

dieghernan opened this issue Aug 6, 2021 · 9 comments · Fixed by #228
Assignees

Comments

@dieghernan
Copy link
Member

dieghernan commented Aug 6, 2021

Hi @antagomir

As you know, I developed giscoR as a kind of spatial spin-off of eurostat.

Recently I have been working on improving the cache dir management (previously set as options(gisco_cache_dir = "path"), similar to the desing on eurostat). Now the cache is set by storing a persistent path using rappdirs::user_cache_dir(), that is loaded as an environment variable at the beginning of the session.

That means that options() command is not needed, in fact the users don't have to worry about the cache any more, that is managed internally by the package. The cache dir can be modified persistently with a custom function (see https://dieghernan.github.io/giscoR/reference/gisco_set_cache_dir.html for Details).

Source code: See https://github.com/dieghernan/giscoR/blob/master/R/gisco_cache.R

If this is something you would like to explore for implementing it also in your package ping me and I could start to have a look on it.

Regards

@antagomir
Copy link
Member

Hi - that would be great, could you consider a PR?

Also, always welcome to host giscoR in association with rOpenGov for added synergies. Otherwise, we should cross-link in eurostat pkg at least.

@pitkant
Copy link
Member

pitkant commented Aug 6, 2021

Yes, maybe changing from options("eurostat_cache_dir" = [DIRECTORY_OF_CHOICE]) to rappdirs would better adhere to Hadley's interpretation of CRAN policies to not change options without user's consent and be more explicit for the end user.

Though it must be said that in the current implementation this getOption("eurostat_cache_dir") is assumed to be NULL if the user has not explicitly stated another directory. I think most users use the default, which uses the base R tempdir() (more precisely file.path(tempdir(), "eurostat"))

I don't see any problem in changing this behaviour, maybe moving cache functionality out of get_eurostat() and get_eurostat_geospatial() (and rewriting clean_eurostat_cache() function) would make the code better organised

@dieghernan
Copy link
Member Author

Great feedback @pitkant , I would try to prepare a PR tackling all those comments.

@antagomir I am considering your proposal:

Also, always welcome to host giscoR in association with rOpenGov for added synergies.

Could you please provide more information on this? There are two main topics I am concerned with when transferring repos to organisations:

  • Ownership and access rights to the repo after transferring it.
  • Integrations with third parties, specifically codecov (that I use for code coverage) and most important zenodo, that provides me with DOIs. Other organisations (as rOpenSpain) didn’t have any problems on activating those services for my repos.

Regards

@antagomir
Copy link
Member

Re: potential transfer:

  • Ownership & access: we can grant the full admin rights to the repo (the admins of the ropengov github organization also have admin rights to all repos, but these would be typically used only in exceptional circumstances)
  • We have had no problems with codecov & zenodo, and these have been used

Interesting to learn about rOpenSpain, this was new!

@dieghernan
Copy link
Member Author

Good then! If you grant me access happy to join rOpenGov!

Regarding rOpenSpain, is a community of R package developers following the spirit of rOpenSci (the motto is "rOpenSci is our form, Spanish public data our matter"). There is a collection of package that extracts Spanish public data, the webpage is in Spanish:

https://ropenspain.es/

See the packages:

https://ropenspain.es/paquetes/

And the r-universe (this one on English):

https://ropenspain.r-universe.dev/ui

I have a couple of packages included. I also am the webmaster and I developed our pkgdown template. See one of my packages, that leverages on giscoR;

https://ropenspain.github.io/mapSpain/

@antagomir
Copy link
Member

antagomir commented Sep 7, 2021

The rOpenSpain shares a lot in common, the starting point for us was Finnish context but there are now many packages with a cross-border coverage. The mapSpain seems a good match with our geofi pkg for Finnish gis info. It might be useful to look at the possible links more closely later.

@pitkant can you check the transfer details with @dieghernan ?

@pitkant
Copy link
Member

pitkant commented Sep 7, 2021

GitHub's Transferring a repository: Transferring a repository owned by your user account seems to explain the process clearly enough. I will give @dieghernan Owner-level rights in rOpenGov-organization and the transfer should be straightforward after that.

@dieghernan
Copy link
Member Author

Aaaand is done!
https://github.com/rOpenGov/giscoR

@pitkant
Copy link
Member

pitkant commented Sep 7, 2021

Great! We can leave this issue open until you a PR about cache dir functionalities is closed.

As a sidenote related to other issues on geospatial functions (e.g. #222 and #213), I should start looking into how Eurostat-package could be improved (and hopefully made more lightweight) by leaning more on separate package, giscoR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants