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

egib layers #102

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

egib layers #102

wants to merge 2 commits into from

Conversation

gsapijaszko
Copy link

Added egib_layers and egib_layer_download().

R/egib_layer_download.R Outdated Show resolved Hide resolved
R/egib_layer_download.R Outdated Show resolved Hide resolved
@kadyb
Copy link
Owner

kadyb commented Nov 14, 2024

The package assumes that if function returns a spatial object without saving it to disk, it takes the name _get. However, I wonder if it wouldn't be better if this function took multiple counties at once and downloaded them to disk, and the users could load them themselves?

R/egib_layer_download.R Outdated Show resolved Hide resolved
R/egib_layer_download.R Outdated Show resolved Hide resolved
@kadyb
Copy link
Owner

kadyb commented Nov 14, 2024

TODO:

  1. Update NEWS file.
  2. Update the table in the README file.
  3. Change examples and tests to download as little data as possible.
  4. How to reproduce egib_layers dataset?

@kadyb
Copy link
Owner

kadyb commented Nov 14, 2024

Could you add yourself as contributor (ctb) to DESCRIPTION?

@kadyb
Copy link
Owner

kadyb commented Nov 14, 2024

If there are counties with the same name, the function returns an error:

egib_layer_download(county = "grodziski")
#> Error in layers[[1]] : subscript out of bounds

@gsapijaszko
Copy link
Author

The package assumes that if function returns a spatial object without saving it to disk, it takes the name _get. However, I wonder if it wouldn't be better if this function took multiple counties at once and downloaded them to disk, and the users could load them themselves?

Done for multiple counties (and the same layer for all of them), outdir pointed out to current directory per default. A bonus: counties with same names will throw a message only and are taken for download. However, if there is more than one county AND outdir = NULL, only the county is taken.

@kadyb
Copy link
Owner

kadyb commented Nov 14, 2024

I think you should update gsapijaszko:master, then the changes will be visible in this PR.

@kadyb
Copy link
Owner

kadyb commented Nov 14, 2024

I also think that we should mention in the docs about setting timeout in case the servers turn out to be VERY slow.

https://gdal.org/en/latest/user/configoptions.html -- GDAL_HTTP_CONNECTTIMEOUT, GDAL_HTTP_TIMEOUT
https://gdal.org/en/latest/drivers/vector/wfs.html -- need to add &Timeout to URL in function?

@gsapijaszko
Copy link
Author

I also think that we should mention in the docs about setting timeout in case the servers turn out to be VERY slow.

https://gdal.org/en/latest/user/configoptions.html -- GDAL_HTTP_CONNECTTIMEOUT, GDAL_HTTP_TIMEOUT https://gdal.org/en/latest/drivers/vector/wfs.html -- need to add &Timeout to URL in function?

New commit is on the way, took me a while to test it.

Regarding the timeout options - yesterday I have started a vignette for EGiB, have a Troubleshooting section. I will check the links and add a few words when needed. The issue is not with time-out, rather few of the servers are not responding at all, either they returns a garbage instead of proper XML.

G.

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

Successfully merging this pull request may close these issues.

2 participants