SDFIdk / FIRE

🔥 FIRE - FIkspunktREgister
https://sdfidk.github.io/FIRE/
MIT License
4 stars 8 forks source link

hent_punkt smider KeyError når hent_punkter returnerer mere end ét punkt #720

Closed krebslw closed 6 months ago

krebslw commented 8 months ago

Eksempel:

> fire.cli.firedb.hent_punkt("SAND")
KeyError: 'SAND'

Problem: Fejlen opstår fordi hent_punkt kalder hent_punkter med "SAND" som søgestreng, som så returnerer alle punkter hvis ident matcher søgestrengen. Der er i hent_punkter taget højde for at man kan søge på et færøisk eller grønlandsk ident uden at foranstille FO eller GL i søgestrengen, se fire/api/firedb/hent.py, ll. 102-105:

...
or_(
    PunktInformation.tekst == ident,
    PunktInformation.tekst == f"FO  {ident}",
    PunktInformation.tekst == f"GL  {ident}",
    ),
...

hent_punkterreturnerer så en liste med punkterne hvis GNSS-numre er "FO SAND" og "SAND". hent_punkt vælger det første punkt hvilket i dette tilfælde er "FO SAND", og populerer en dict som mapper punktet til dens identer. Derefter slås identen op i dicten med den oprindelige søgestreng "SAND" som key, hvilket giver en KeyError, da dicten kun indeholder identer for punktet "FO SAND".

Jeg har ikke tjekket om problemet kun opstår når der er sammenfald mellem grønlandske, færøske og danske GNSS-numre, men som jeg læser koden, så bør det kun være i disse tilfælde.

Løsningsforeslag: Der findes selvfølgelig måder at omgå dette problem ved at man selv kalder hent_punkter direkte og trækker det korrekte punkt ud eller bruger soeg_punkter som ikke foranstiller FO eller GL. Dog bør funktionen hent_punkt ikke fejle ved gyldigt input.

  1. Sørg for at hent_punkt ikke vælger det første punkt, men derimod prioriterer et punkt hvis ident matcher 1:1.
  2. Sørg for at hent_punkter ikke søger med foranstillet FO eller GL.

Hvis det virkelig er meningen at hent_punkt skal returnere det første punkt i listen, altså at der skal returneres punktet FO SAND når der søges på "SAND" så kræves der en anden løsning.

kbevers commented 8 months ago
  1. Sørg for at hent_punkt ikke vælger det første punkt, men derimod prioriterer et punkt hvis ident matcher 1:1.

Jeg synes den her løsning lyder som den rigtige vej at gå.

Jeg har ikke tjekket om problemet kun opstår når der er sammenfald mellem grønlandske, færøske og danske GNSS-numre, men som jeg læser koden, så bør det kun være i disse tilfælde.

Jeg tror der er enkelte andre situationer hvor det også kan opstå, men det er primært GNSS identerne der volder problemer enkelte steder.

Hvis det virkelig er meningen at hent_punkt skal returnere det første punkt i listen

Der er helt sikkert ikke tænkt så forfærdeligt meget over det - i langt de fleste tilfælde er første punkt i listen det rigtige så det bare været den nemme løsning. Fint at få gjort koden mere robust her!