Bugfix extra Anja

[leymanan]

No description provided.

[ElsLommelen]

Ik had gemerkt dat er nog enkele typo's en layoutfoutjes in mijn branch 'bugfix' zaten, en deze heb ik net opgelost. Dus je haalt best die nieuwe commits binnen in deze branch (eerst even lokaal naar bugfix gaan en pullen, en daarna terug naar deze branch (bugfix_AL) gaan en in de git-terminal git merge bugfix typen).

Vergeet trouwens niet om regelmatig de unittests te runnen, zeker met die check-functies is dit belangrijk want daar verdwijnen de foutboodschappen als je bv. vergeet om bij elke nieuwe foutboodschap te testen of die nog wel waarde NA had (& is.na(.data$field_decay_stage)). Ik weet het, het is een beetje vervelend dat de checks altijd falen zolang er geen release gemaakt is in forresdat en je dus altijd een waarschuwing krijgt en niet enkel als je extra fouten genereert, maar af en toe in tab build op Test duwen en zien of alles nog slaagt, is eigenlijk maar een kleintje.
Misschien best toch wel zorgen dat er in forresdat een release is vooraleer we deze branch mergen? Dat maakt het testen toch wel veel handiger. (Ik had die layoutfoutjes ook niet opgemerkt omwille van die forresdat-gerelateerde error bij de voorbeelden.)

[leymanan]

Ik had gemerkt dat er nog enkele typo's en layoutfoutjes in mijn branch 'bugfix' zaten, en deze heb ik net opgelost. Dus je haalt best die nieuwe commits binnen in deze branch (eerst even lokaal naar bugfix gaan en pullen, en daarna terug naar deze branch (bugfix_AL) gaan en in de git-terminal git merge bugfix typen).

OK! Is gebeurd.

Vergeet trouwens niet om regelmatig de unittests te runnen, zeker met die check-functies is dit belangrijk want daar verdwijnen de foutboodschappen als je bv. vergeet om bij elke nieuwe foutboodschap te testen of die nog wel waarde NA had (& is.na(.data$field_decay_stage)). Ik weet het, het is een beetje vervelend dat de checks altijd falen zolang er geen release gemaakt is in forresdat en je dus altijd een waarschuwing krijgt en niet enkel als je extra fouten genereert, maar af en toe in tab build op Test duwen en zien of alles nog slaagt, is eigenlijk maar een kleintje.

Inderdaad, ik ga er proberen opletten!
Net gerund, één ding al gecorrigeerd, maar voor tweede failure zou ik aanpassingen in testdb moeten doen.
En ik denk voor de volgende ook.
(check_data_plotdetails: ik heb daar enkele extra vwdn ingestoken: als er geen survey is geweest, dan is het niet erg dat er NA's zijn voor rA1/2/3/4 en fieldteam)

Misschien best toch wel zorgen dat er in forresdat een release is vooraleer we deze branch mergen? Dat maakt het testen toch wel veel handiger. (Ik had die layoutfoutjes ook niet opgemerkt omwille van die forresdat-gerelateerde error bij de voorbeelden.)

OK! Ben nu weer met wat andere dringendere zaken bezig, maar ik zet het op mijn to do voor begin volgend jaar!

[ElsLommelen]

(check_data_plotdetails: ik heb daar enkele extra vwdn ingestoken: als er geen survey is geweest, dan is het niet erg dat er NA's zijn voor rA1/2/3/4 en fieldteam)

Als je er bewust voor kiest om voor een bepaalde fout niet meer weer te geven in de output, dan pas je dat eigenlijk beter in de unittests zelf aan. Best voer je de test dan nog wel uit, en dan zou het resultaat een lege tabel moeten zijn terwijl die bewuste records nog wel in testdb zitten. (Voordeel is dat je zo ook effectief test dat die foutmelding weg is en dat je ook test dat ze niet onvoorzien op een andere plaats tevoorschijn komt, terwijl het aanpassen in de databank ervoor zou zorgen dat voor een bepaald type record niet meer getest wordt bij welke foutmelding het eventueel tevoorschijn komt. Dus je mag die testdb evt. wel aanvullen met extra records, maar ik zou bestaande records niet wegdoen of aanpassen want het blijft altijd nuttig om ze mee te testen, ook als je wil dat ze 'geen' fout genereren. Je zal zien, over x jaar maak je ooit eens een verandering in de code waardoor ze plots terug tevoorschijn komen en de unittests doen falen, en dan ben je blij dat je dit op die manier merkt en niet nadat de nieuwe versie in gebruik is...)

(Dit heeft uiteraard geen haast, neem het gerust terug op als je tijd hebt, maar ik zag het een tijdje geleden passeren en nu had ik een klein beetje tijd om die aanpassing te doen en je hier even attent op te maken.)

[ElsLommelen]

Misschien hier toch best even documenteren waarom je het minimum volume gelijkstelt aan 1 dm³? Dat je wil vermijden dat een volume negatief is (de 0 die er oorspronkelijk stond), lijkt nog vrij logisch, maar hier lijkt het me wel relevant om aan te geven waarom het volume niet 0 mag zijn (zelfs niet als dbh_mm 0 is of kan dat niet?) en waarom net dat volume.
(Dat mag gewoon met een lijntje commentaar boven deze lijn code als het niet relevant is voor de gebruikers van het package om te weten dat dit gebeurt, anders wordt het uiteraard beter ergens gedocumenteerd waar een gebruiker het ook kan lezen bij gebruik van de functie.)