inbo / forrescalc

Calculation of aggregated values on dendrometry, regeneration and vegetation of forests, starting from individual tree measures from Fieldmap
https://inbo.github.io/forrescalc/
GNU General Public License v3.0
0 stars 0 forks source link

quality control #1

Open ElsLommelen opened 4 years ago

ElsLommelen commented 4 years ago

a.o. functions that give the most extreme differences between years (f.i. based on functions calculate_dendro_plot, calculate_dendro_plot_species and calculate_regeneration_plot_height_species)

ElsLommelen commented 3 years ago

check_data_trees(): periode 3 nog toevoegen

leymanan commented 2 years ago

Bij regeneration obv veld GameImpactRegObserved (uit PlotDetails) checken of er geen foute nullen in databank zitten (0 waar NA zou moeten zijn)

ElsLommelen commented 4 months ago

@leymanan Ik vermoed dat dit overeenkomt met je laatste vraag?

In verband met mijn allereerste opmerking checken we enkel de diameter en hoogte bij de metingen zelf, dus we testen niks op de resultaten gegenereerd door de calculate-functies. Is dit nog nodig of nuttig, of is het voldoende als die resultaten van individuele bomen gecheckt zijn op juistheid?

leymanan commented 4 months ago

@leymanan Ik vermoed dat dit overeenkomt met je laatste vraag?

inderdaad ;-)

In verband met mijn allereerste opmerking checken we enkel de diameter en hoogte bij de metingen zelf, dus we testen niks op de resultaten gegenereerd door de calculate-functies. Is dit nog nodig of nuttig, of is het voldoende als die resultaten van individuele bomen gecheckt zijn op juistheid?

goede vraag, ik zou veronderstellen dat calculate functies doen wat ze moeten doen, je kan ze toch enkel checken door zelfde code opnieuw te runnen, niet? Dus volgens mij voldoende om enkel diameter en hoogte van de metingen zelf te checken

ElsLommelen commented 4 months ago

goede vraag, ik zou veronderstellen dat calculate functies doen wat ze moeten doen, je kan ze toch enkel checken door zelfde code opnieuw te runnen, niet?

Euh, hier was mijn vraag eerder specifiek over de outliers bij de uitkomst bekijken, wat misschien inderdaad niet zoveel zin heeft. Maar om specifiek de werking van de calculate-functies te testen (of andere functies in het package die een welbepaalde berekening maken startend van een input), kan je wel unittests schrijven: je runt de functie met een bepaalde input (dataframe), en je test of de output hetzelfde is als wat je zou verwachten. (Dus je rekent dat bv. een keertje manueel uit, en dat resultaat vergelijk je telkens met de output bij het runnen van de functie.) Naast pure getalmatige invoer kan je op deze manier ook testen of hij juist reageert als er een NA waarde ingevoerd wordt, of hij de juiste error geeft als er bv. tekst i.p.v. een getal ingevoerd wordt, of de juiste kolomnamen teruggegeven worden,...

Dit is wel wat werk als we dat grondig willen doen voor alle functies, maar dit heeft meerdere voordelen:

Hoewel ik het heel nuttig vind, vrees ik dat ik voorlopig geen tijd ga hebben om dat meteen grondig te doen voor alle functies (dat zijn er intussen al redelijk veel!). Maar met het testdatabankje is het wel weinig werk om even enkele tests te maken die echt nuttig lijken. En dan stel ik vooral voor dat we bij elke fout die we vinden, meteen een unittest schrijven die specifiek voor die fout test. (Zo doe ik het bij andere packages ook.) En als ik later op het jaar nog wat tijd heb, kan ik dat wel systematisch aanvullen. Wat ik ook vaak doe, is in het package bij een PR als voorwaarde op te leggen dat die unittests een groter percentage code moeten coveren als vooraf, en zo verplicht ik mezelf (of iedereen die aan het package werkt) om bij toevoegingen aan de code een extra unittest te schrijven. Dus ik stel voor om binnenkort toch een eerste versie van het package af te werken, zonder of met weinig unittests, en dan die unittests de komende jaren stap voor stap verder uit te werken in functie van de noden en lichtjes dwingen door die voorwaarde dat die coverage van code door unittests moet toenemen. (Specifiek voor unittests is er trouwens nog een apart issue)

ElsLommelen commented 2 months ago

Bij regeneration obv veld GameImpactRegObserved (uit PlotDetails) checken of er geen foute nullen in databank zitten (0 waar NA zou moeten zijn)

Ik las deze vraag nog eens opnieuw, en vroeg me dan af: moet ik behalve het feit dat het geen NA mag zijn als GameImpactRegObserved 10 (yes) is, ook niet testen of het ten onrechte een waarde (of 0) heeft gekregen als GameImpactRegObserved 20 (no) is?

leymanan commented 2 months ago

Ik las deze vraag nog eens opnieuw, en vroeg me dan af: moet ik behalve het feit dat het geen NA mag zijn als GameImpactRegObserved 10 (yes) is, ook niet testen of het ten onrechte een waarde (of 0) heeft gekregen als GameImpactRegObserved 20 (no) is?

ja, dat mag eigenlijk wel, goed idee!

ElsLommelen commented 2 months ago

voila, bij deze toegevoegd. Dan is dit issue eigenlijk opgelost, behalve de ene commentaar die uitweidt over de unittests.