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

Tools for lifecycle updates and removed buildings #19

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

zidel
Copy link

@zidel zidel commented May 18, 2022

@NKAmapper, hørtes ut som du har skrevet noe av dette allerede, men lager en PR på det jeg har, så kan du plukke ut det som er nyttig

@NKAmapper
Copy link
Owner

Supert. Jeg skal få sett på dette om noen dager.

@hansfn
Copy link

hansfn commented Jun 7, 2023

@NKAmapper Siden denne PR-en ikke endrer på eksisterende verktøy utenom filter_buildings.py, så hadde det vært fint om den ble merget. Dette virker som nyttige verktøy.

@zidel Jeg likte egentlig at filter_buildings.py spyttet ut

Loaded ... buildings
Loaded ... unique references from OSM
Writing ... buildings missing from OSM

Kunne noe av dette vært bevart?

@NKAmapper / @zidel: Som bruker så er det ulogisk at filter_buildings.py skal ha --municipality ID mens alle andre skript har <municipality> (altså navn). Noe dere kunne fikset samtidig?

@NKAmapper
Copy link
Owner

Det var usikkerhet her om metoden for å finne nye og fjernede bygninger er robust nok.
Hva er erfaringen her, @zidel ?
Finnes det alternative metoder?

@zidel
Copy link
Author

zidel commented Jun 10, 2023

Lenge siden jeg har sett på dette nå, men kan ikke huske at det var noen problemer med verktøyene isolert sett, sammenlikningen på bygningsnr burde fungere bra. Det jeg vet jeg hadde problemer med var bygninger som ble borte fra datasettet (og derfor ble filtrert som removed), men ser at de eksemplene jeg husker har kommet tilbake med samme bygningsnr som i OSM, så mulig det bare var noe midlertidig eller relatert til bytte av datakilde.

Magefølelsen når jeg ser på data fra Lillestrøm igjen er at det ser ut som en god kilde for å finne oppdateringer.

@hansfn skal se på kommunenavn i stedet for id, building_merge gjør allerede konverteringen fra navn til id så det burde ikke være noe problem. Er det antall bygninger du savner? Tror jeg ofret det når jeg prøvde meg på unit testing, men det burde ikke være noe problem å få til begge samtidig.

Martin Nyhus added 5 commits June 10, 2023 20:34
Treat municipality id as a string since type=int doesn't work when the
leading zero is important.
Move the data parsing and filtering steps to functions that can be unit
tested. Also adds a few tests for the filtering code to put the testing
infrastructure in place.
Adds a tool for finding buildings that are planned or under construction
in OSM, but are finished in the cadastral data. Since IG doesn't mean
that construction has actually started, planned and construction are
treated as the same thing.
Adds a tool for finding OSM buildings that have been removed from the
cadastral registry. From testing in Lillestrøm this is unfortunately not
a reliable indicator of the building actually having been removed, so
the output format is intentionally not suitable for automatic uploading.
@zidel
Copy link
Author

zidel commented Jun 11, 2023

  • Rebase på siste main
  • La til antall bygninger i output fra alle tre filtrene
  • Alle tre kan nå ta kommune med navn (unntatt Våler som det finnes to av)

@NKAmapper
Copy link
Owner

@hansfn Vil du kjøre et par tester og se på resultatene for nye/fjernede bygninger?

@hansfn
Copy link

hansfn commented Jul 15, 2023

Endelig fant jeg tid til å teste litt.

  • find_removed.py: Testet i Molde. Fant flere bygninger som var revet pga. nye vei og andre prosjekt. Svært nyttig syns jeg.
  • find_lifecycle_updates.py: Testet i Molde og Oppdal. Verktøyet fant ingen bygninger med oppdateringer. Jeg forventet egentlig å finne noen - hvor nye bygninger var plassert over gamle, men jeg husker ikke i farten om jeg har rettet dette tidligere. (Jeg sjekket ikke koden så jeg vet ikke hva den egentlig ser etter.)

Uansett, jeg vil svært gjerne at find_removed.py blir inkludert. Jeg antar find_lifecycle_updates.py også gjør jobben selv om jeg ikke fikk testet det.

Noen UX forbedringer (som jeg helt klart kan fikse selv i en PR om dere ønsker):

  • Legg til beskrivelse av de nye verktøyene README.md
  • building2osm.py og building_merge.py bør støtte "h/help" valg så man ikke må lese README.md for å vite bruk.
  • find/filter* verktøyene bør oppføre seg seg som build*
    • Bytt ut --municipality MUNICIPALITY valg med MUNICIPALITY argument
    • Input JSON: "If not specified, the import file for the municipality will be loaded"
    • Hjelpmeldingen bør si at INPUT og OUTPUT er JSON filer.

@zidel
Copy link
Author

zidel commented Jul 15, 2023

@hansfn, find_lifecycle_updates.py ser etter planned:building eller building=construction i OSM hvor Matrikkelen mener bygget er tatt i bruk, så det er et relativt smalt usecase men nyttig for å hindre at ting blir værende som construction i evigheter.

@hansfn
Copy link

hansfn commented Jul 15, 2023

Takk for forklaringen. Viser behov for litt info i Readme ;-)

Helt greit for meg at PR-en merges uten at alle UX tingene blir håndtert.

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.

None yet

3 participants