inbo / n2khab

R package with preprocessing functions and standard reference data for Flemish Natura 2000 (N2K) habitat (HAB) analyses
https://inbo.github.io/n2khab
GNU General Public License v3.0
2 stars 1 forks source link

Update read_habitatmap, read_hm_stdized ,read_hm_terr for hm 2020 #139

Closed cecileherr closed 3 years ago

cecileherr commented 3 years ago

Updated: 

For read_habitatmap() : added a basic control of the checksums to ensure that the used version for habitatmap corresponds to the version of habitatmap_stdized stored under n2khab_data (useful since read_habitatmap_stdized() is being called within read_habitatmap())

About the errors in geometry in habitatmap and the processed versions (Ring Self-intersections, see also https://github.com/inbo/n2khab-preprocessing/pull/50#issuecomment-815528848) : Despite the errors, geometric operations are possible with these layers: e.g. an intersection seems to produce the expected results (more info). Using st_make_valid has not been implemented in the read_ functions (my guess is that it is not needed with the present version of sf) but it could be an option for read_hm_terr and read_hm_stdized since:

Using st_make_valid for the raw version of the habitatmap takes longer though (1 min). Might be better not to use it since read_habitatmap() takes already quite long to run.

florisvdh commented 3 years ago

@cecileherr thank you, will review the function updates asap!

Responding already to st_make_valid():

it could be an option for read_hm_terr and read_hm_stdized since

If you would like to implement st_make_valid() for habitatmap_terr and habitatmap_stdized, then I suggest to update the data sources themselves in the PR where they're made (see https://github.com/inbo/n2khab-preprocessing/pull/50#issuecomment-838494498), not doing it in the reading functions.

Maybe you mean to provide it through an optional argument, but I think this would not help standardization either.

cecileherr commented 3 years ago

@cecileherr thank you, will review the function updates asap!

Responding already to st_make_valid():

it could be an option for read_hm_terr and read_hm_stdized (...)

About using st_make_valid() while creating the data sources: we decide NOT to do it for the moment.

florisvdh commented 3 years ago

@cecileherr were you ready with this PR? If so, I'll just await the checks running here and then I'll merge.

cecileherr commented 3 years ago

@cecileherr were you ready with this PR? If so, I'll just await the checks running here and then I'll merge.

Yes, this PR was ready (all the needed changes have already been made in PR#140)