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

Update functions replace_existing_points and draw_sample #6

Merged
merged 14 commits into from
Feb 19, 2024

Conversation

EmmaCartuyvels1
Copy link
Collaborator

De functies zijn aangepast zoals we besproken hadden en de pipeline loopt.

Copy link
Collaborator

@hansvancalster hansvancalster left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Vooral een paar vragen die nog moeten uitgeklaard worden.

@EmmaCartuyvels1
Copy link
Collaborator Author

Ik ga ook de markdown van de zandstreek al updaten, op die manier kunnen we ook visueel de overlap met bestaande punten controleren.

@EmmaCartuyvels1
Copy link
Collaborator Author

Markdown is up to date en steekproefontwerp_zandstreek.html staat in de map MBAG - 3_statistiek - outputs

@hansvancalster
Copy link
Collaborator

Markdown is up to date en steekproefontwerp_zandstreek.html staat in de map MBAG - 3_statistiek - outputs

Merci voor het overzicht. Een paar observaties:

  • er zijn grote landbouwblokken waar maar weinig paden doorlopen
  • lang niet alle paden lijken op OSM aanwezig
  • er is een (wellicht drukke) expressweg ingekleurd als "unclassified"

Dat duidt allemaal enkel op mankementen in OSM. Ik denk niet dat we stappen moeten ondernemen om hier iets aan te doen. De zandstreek is groot genoeg om geschikte punten te vinden. De steekproef zelf ziet er wel OK uit.

Comment on lines +892 to 895
mutate(steekproeftrekking = ifelse(popsize < 2000,
round(popsize / 5),
round(popsize / 20))) %>%
select(-c(openheid_sbp, targetsize, excess)) %>%
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Deze wijziging is nog niet gereflecteerd in de begeleidende tekst. Kan je dat aanpassen?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is aangepast!

@hansvancalster
Copy link
Collaborator

In Hoofdstuk 3.2 loopt er iets mis in de code. In de figuren staat in de legende NA. Kan je daar ook eens naar kijken? Volgens mij gaat het wellicht mis wanneer het object balansvergelijking berekend wordt. De bedoeling van die berekening en figuren is om na te gaan of de steekproef, bij verschillende steekproefgroottes, een goede weerspiegeling is van wat er in het volledige steekproefkader zit voor de variabelen waarmee we rekening houden bij de trekking. Dit zijn X, Y, opp-aandeel SB. We willen dus dat de boxplots er ongeveer hetzelfde uit zien. Ook de begeleidende tekst daar zou moeten aangepast worden, want ze beschrijft volgens mij nu de resultaten voor de Leemstreek of zandleemstreek.

Kan je ook gelijkaardige markdown maken voor de Polders en de Kempen?

@wlangera
Copy link
Collaborator

@hansvancalster in 2.3.10-2.3.11 excluderen we autosnelwegen (highway = motorway) en afritten (highway = motorway_link) met een buffer van 100 m om paralelle wegen uit te sluiten. De expressweg Knokke-Antwerpen is geen autosnelweg (of toch niet volledig), want daar staan nog verkeerslichten. Deze weg valt op osm onder highway = trunk. Hoewel ze hier wel wel op "korte" termijn en autosnelweg willen van maken, maar dit zal nog even duren denk ik aangezien stukken onteigend zullen moeten worden ...

Dus het is niet de expressweg die highway = unclassified is, wel enkele paralelle wegen er vlak langs aangezien die niet zijn verwijderd door een buffer omdat de expressweg geen highway = motorway of highway = motorway_link is.
Tellers zullen dus niet op de expressweg moeten tellen maar wel op de paralelle wegen (ik zie 3 punten in de eerste 50 punten van het steekproefkader?).

Ik denk dus ook wel dat dit ok is, maar wou dit nog even verduidelijken.

@hansvancalster
Copy link
Collaborator

Ik denk dus ook wel dat dit ok is, maar wou dit nog even verduidelijken.

Merci om daar even naar te kijken. Dat is in elk geval geruststellend.

@wlangera
Copy link
Collaborator

Kan je ook gelijkaardige markdown maken voor de Polders en de Kempen?

We zitten met de volgende branches:
main <- overige_landbouwstreken <- replace_existing_points

De huidige PR is overige_landbouwstreken <- replace_existing_points. Deze had Emma gemaakt op mijn aanraden (verbeter mij indien ik mis ben @EmmaCartuyvels1 ) om de functie te herschrijven die de overlap met bestaande punten na te gaan in de targets pipeline.

Dus misschien moeten we in deze PR focussen dat de targets pipeline goed werkt o.b.v. de zandstreek en dan in een volgende PR, nl. main <- overige_landbouwstreken, de markdowns zandstreek, polders en kempen nakijken?
Kwestie van het wat overzichtelijk te houden, maar het is om het even voor mij.

@EmmaCartuyvels1
Copy link
Collaborator Author

Ik heb sectie 3.2 aangepast en enkel de overlappende punten op de kaart. Nieuwe markdown vinden jullie in de drive.

@hansvancalster
Copy link
Collaborator

In de markdown zie ik
image

maar deze bestanden zie ik niet bij "changed files". Kan het zijn dat je deze nog moet committen en pushen?

@hansvancalster
Copy link
Collaborator

In de markdown zie ik image

maar deze bestanden zie ik niet bij "changed files". Kan het zijn dat je deze nog moet committen en pushen?

Ik zie nu toevallig ook dat de naam van dat bestand nog niet is aangepast. Het moet zandstreek zijn ipv leemstreek.

@EmmaCartuyvels1
Copy link
Collaborator Author

In de markdown zie ik image
maar deze bestanden zie ik niet bij "changed files". Kan het zijn dat je deze nog moet committen en pushen?

Ik zie nu toevallig ook dat de naam van dat bestand nog niet is aangepast. Het moet zandstreek zijn ipv leemstreek.

Er staat niks voor meer open om te pushen, die bestanden zijn wel aangemaakt. Ik zie ze ook niet in de gitignore staan 🤔
Sowieso worden die laatste bestanden pas aangemaakt nadat Johannes een controle heeft uitgevoerd, dus ik zou daar hier nog niet op focussen.

Copy link
Collaborator

@hansvancalster hansvancalster left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Deze PR lijkt me in orde 👍 Later nog even uitzoeken hoe het juist zat met wegschrijven van steekproef en steekproefkader via git2rdata

@EmmaCartuyvels1 EmmaCartuyvels1 merged commit 705a442 into overige_landbouwstreken Feb 19, 2024
1 check failed
@EmmaCartuyvels1 EmmaCartuyvels1 deleted the replace_existing_points branch February 19, 2024 14:02
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.

3 participants