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

Standardize code formatting with black #2

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

Conversation

harahu
Copy link
Contributor

@harahu harahu commented Sep 16, 2022

I had planned to dive deeper into the internals of this tool, and perhaps contribute to it as a hobby project. The first thing I do when interacting with python code, is to format it with black, in order to make it easier to read.

The idea behind black is that it exposes very little in the way of configuration options, meaning that code that is formatted with it, usually looks "the same". By unifying on a common, tool-enforced code format, it makes it easier for everyone to understand and contribute to code. Python formatted with black is fast becoming a standard in the open source community, so I thought I perhaps should contribute this formatting back to you.

The size of the diff in this PR might seem intimidating, but I assure you, there's no behavioral changes, and the AST should be intact.

@NKAmapper
Copy link
Owner

Koden er litt hjemmesnekret ja ... :)
Det du foreslår er jo bedre, men er litt usikker på dette. Jeg gjenbruker mye kode på tvers av repos, og endring av format ett sted kan blir upraktisk for meg.

Fint om du vil bidra selvfølgelig. Hva har du i tankene?

Har selv tenkt at viktigste mangel nå er mulighet i n50merge.py til å dele opp importjobben i mindre kvadranter, slik som i splitterOsm.py.

@harahu
Copy link
Contributor Author

harahu commented Sep 19, 2022

Det du foreslår er jo bedre, men er litt usikker på dette. Jeg gjenbruker mye kode på tvers av repos, og endring av format ett sted kan blir upraktisk for meg.

Det forstår jeg veldig godt. Alternativet da er jo å black-formatere alle prosjektene du vil dele kode med, som kanskje er litt mye å be om. Men, om du liker formateringen, så er jo det muligens noe å vurdere. Det er ikke vanskeligere enn:

pip install black
black ./

Dette er dog ditt personlige prosjekt, så har ikke lyst å pushe for hardt med alskens praksiser. Det viktigste er jo at man har det gøy.

Fint om du vil bidra selvfølgelig. Hva har du i tankene?

Jeg begynte å se på muligheten for å importere N50-data for Aurland og Lærdal kommuner, men hadde lyst å generere helt ferske osm-filer. Jeg kom over dette verktøyet, men fant ut at det gikk sakte, som advart, om du vil kalkulere retning på bekker og elver. Jeg hadde derfor et håp om å se litt på performance ift. kalkulering av retning på elver og denslags. Men oppdeling av importer er naturligvis også interessant.

Jeg jobber som machine learning engineer, og jobber stort sett bare med python, så er erfaren med språket. Men kartdata er nytt for meg å jobbe med, så jeg ser også på dette som en mulighet for å lære meg litt om om et nytt domene. Det er nok derfor litt åpent hva som kommer ut av dette. Det første jeg gjør, for å få et intimt og godt kjennskap til koden, er rett og slett å skrive den om. Jeg så det ble brukt en del global state. Ikke noe i veien med det sånn egentlig, men det gjør det vanskelig å forstå kodeflyten for meg som utenforstående. Derfor prøver jeg å omstrukturere den til å unngå dette. Legger også til type-annotasjoner, slik at jeg får dokumentert for meg selv hvordan dataflyten ser ut. Først etter dette, tror jeg, vil jeg begynne å se etter algoritmisk forbedringpotensiale.

Har lekt litt med å bruke et eksternt bibliotek for å forenkle CLIet litt, får se om det blir noe bra. Har også begynt å eksperimentere med å lage en automatisk bygget zipapp, ved hjelp av github actions, for å få litt trening i det. Mye på en gang med andre ord.

Skal prøve å ikke spamme deg med PR-er før jeg har noe som jeg vet fungerer godt.

@NKAmapper
Copy link
Owner

Det høres ut som det blir omfattende endringer. Vi får se om dette bør ha sitt eget repo.

Det er to måter å få høyder raskere:

  1. Api'et til Kartverket tillater høyde-oppslag på opptil 50 koordinater per kall. Kanskje dette vil gi raskere svar enn ett punkt per kall. Har ikke testet ennå.
  2. Laste ned relevant område fra den nasjonale høydemodellen og gjøre oppslagene lokalt på PC. Det skal gå mye raskere. Et kodeeksempel fra Kartverket nedenfor. WCS-tjenesten.

Ellers er erfaringen at det går mest tid i selve importprosessen/flettingen, så større effekt av smarte løsninger for effektiv arbeidsflyt der enn litt raskere generering av importfilen.

Kodeeksempel:

# conda install -c conda-forge owslib 

from owslib.wcs import WebCoverageService
#endpoint='https://hoydedata.no/arcgis/services/dtm1_33/ImageServer/WCSServer?request=GetCapabilities&service=WCS'
endpoint='https://wms.geonorge.no/skwms1/wcs.hoyde-dtm1_33?request=GetCapabilities&service=WCS'
identifier='dtm1_33_wcs'
wcs = WebCoverageService(endpoint,version='1.0.0',timeout=60)

#f=open("Nivbasen_Fastmerker_NN2000.gob","r")
#lines=f.readlines()
#f.close()

point_E = 219144.1672551394# 214506.11  #F38AN004
point_N = 6802946.507040344# 6549282.63 
boxdim = 1
# NN2000 = 947.67 m at Location

#bbox = (point_E-boxdim, point_N-boxdim,point_E+boxdim, point_N+boxdim) 
bbox = (point_E, point_N,point_E+boxdim, point_N+boxdim) # Kun 1 punkt
# print (bbox)
output = wcs.getCoverage(identifier=identifier,bbox=bbox,crs='EPSG:25833',format='GeoTIFF', resx=1, resy=1) 
#print(output.read().squeeze())
out = open('tryvann.tiff','wb')
out.write(output.read())
out.close()
import matplotlib.pyplot as plt
import rasterio
with rasterio.open('tryvann.tiff') as src:
    v = src.read()
#plt.imshow(v[0]); plt.colorbar(); plt.show()

print(point_E,point_N,v.squeeze())
# Riktig svar (FM) er: 219144.1672551394 6802946.507040344  947.67

@harahu
Copy link
Contributor Author

harahu commented Sep 22, 2022

Takker for nyttig innsikt og spor jeg kan følge opp!

Det er helt klart aktuelt å gjøre dette arbeidet i et eget repo. Om/når jeg har noe jeg er fornøyd med, så kan jeg jo høre om repoet kan hostes hos osmno? Usikker på hvor mye aktivitet det er der om dagen.

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

2 participants