inbo / dhcurve

An R package for automated modelling of diameter-height relations for trees
https://inbo.github.io/dhcurve
GNU General Public License v3.0
0 stars 0 forks source link

Afwerking package #44

Closed ElsLommelen closed 2 years ago

ElsLommelen commented 3 years ago

In deze PR worden checks en een website in INBO-stijl toegevoegd, gebruik makend van het package checklist.

(Ik zet deze branch voorlopig nog op draft: ik zie dat het vignet nog ontbreekt op de website,... Ik werk dit stap voor stap af, maar bekijk gerust alvast. Houd er wel rekening mee dat ik hier van de develop-branch vertrokken ben, dus de aanpassingen van de voorbije weken zitten hier nog niet in, wat betekent dat het nog tamelijk wat foutmeldingen geeft en verouderde teksten. Dus best puur naar de layout van de website kijken (en generieke info zoals de kolom rechts), en de items die ik hierboven opsom.)

ElsLommelen commented 3 years ago

Het lijkt erop dat dit ongeveer in orde is, de foutmelding in de check is te wijten aan falende unittests, wat opgelost is in de branch update-code. Dus zodra update-code gemerged is met de develop, kunnen we deze PR nog even checken en mergen.

ElsLommelen commented 3 years ago

draft enkel ongedaan gemaakt om tests te kunnen doen, dus nog niet klaar voor review

ElsLommelen commented 3 years ago

Intussen heb ik de belangrijkste issues opgelost, de automatische checks voor zover mogelijk op punt gesteld en het kleurtje van de website aangepast (bekijk zeker of die goed is via pkgdown::build_site(), deze komt (hopelijk ;-) ) online zodra we naar de master mergen).

leymanan commented 3 years ago

@ElsLommelen: moet ik pkgdown::build_site() in shell ingeven? Of in een gewoon R script?

leymanan commented 3 years ago

ivm kleur: Leen en ik hebben ook enkele markdown-rapporten op de website van ANB geplaatst, met groene kleur (zelf bepaald). (zie bv. "https://www.natuurenbos.be/vlaamse-bosinventaris/bosareaal") De algemene website heeft al geen groene kleur meer: "https://www.natuurenbos.be/beleid-wetgeving/natuurbeheer/bosinventaris"

Onze groene kleur komt gewoon uit "word": image Ik heb héél ambachtelijk een logo + lijn gecreëerd. Was snelste manier ;-)

Voor mij is dat goed genoeg. Ev. kan wel dat groen leeuwtje toegevoegd worden als logo Vlaanderen_is_natuur_1 Entiteitlogo_ANB_1

ElsLommelen commented 3 years ago

@ElsLommelen: moet ik pkgdown::build_site() in shell ingeven? Of in een gewoon R script?

In een R-script op het moment dat je de repo van dhcurve open hebt.

ElsLommelen commented 3 years ago

Ik zal even je reactie op m'n eerste poging afwachten, en daarna het leeuwtje en het logo nog toevoegen, samen met je suggesties. (Ik heb de groene kleur gehaald van een website waar de kleuren van alle vlaamse stijlen opgelijst waren (met RGB-waardes), daar stond behalve groen ook bruin en blauw voor ANB, als ik me niet vergis.)

Wat mij een beetje stoort, is dat het donkergroen bij de tekst amper te onderscheiden is van zwart, waardoor niet goed zichtbaar is onder welke tekst een link verborgen is. Maar ik laat het aan u om hiervoor een betere suggestie te geven. ;-)

leymanan commented 3 years ago

Ziet er goed uit! Ik heb voorkeur voor groen, maar ik snap wat je bedoelt met die linken. Kan je dat ev. omwisselen: lichtgroen en donkergroen als je erover gaat met muis? Of gewoon standaard blauw voor die linken?

Op website van ANB is het nu eerder grijs met roze, maar veel kans dat dat over paar jaar weer anders is ;-)

ElsLommelen commented 3 years ago

Voila, kleurtje is al omgewisseld (dat lichte is eigenlijk wel een INBO-kleurtje ;-) ). Toevoegen logo's en poster is voor maandag.

leymanan commented 3 years ago

👍☺

Op vr 26 mrt. 2021 om 18:57 schreef ElsLommelen @.***>:

Voila, kleurtje is al omgewisseld (dat lichte is eigenlijk wel een INBO-kleurtje ;-) ). Toevoegen logo's en poster is voor maandag.

— You are receiving this because your review was requested. Reply to this email directly, view it on GitHub https://github.com/inbo/dhcurve/pull/44#issuecomment-808412620, or unsubscribe https://github.com/notifications/unsubscribe-auth/AGKXN5CJ54KDB2FCEBB5HATTFTDI5ANCNFSM4U3CVLYQ .

--

Anja Leyman

Expert Cel Beheerplanning en Monitoring

//////////////////////////////////////////////////////////////////////////////////////////////////////////////////////

Vlaamse overheid

AGENTSCHAP NATUUR & BOS

Standplaats Instituut voor Natuur- en Bosonderzoek (INBO) Gaverstraat 4, 9500 Geraardsbergen T: +32 495 14 90 60 E-mail: @. @.>*

www.natuurenbos.be http://www.natuurenbos.be/

De inhoud van dit bericht en eventuele bijlage(n) verbinden het Agentschap voor Natuur en Bos niet, zolang niet bevestigd door een geldig ondertekend document

ElsLommelen commented 3 years ago

@ThierryO Ik zou graag bij de auteurs bij copyrighthouder ANB de tekst vervangen door de naam in de huisstijl, zoals bij het checklist-package, maar dit lukt me niet. Ik heb al vanalles geprobeerd qua syntax in _pkgdown.yml (zonder te committen), en ik zie ook niet waar ik het elders zou kunnen definiëren. Enig idee wat ik over het hoofd zie?

ThierryO commented 3 years ago

Commit jouw poging van _pkgdown.yml, dan kan ik er eens naar kijken. Mijn telepathie skills zijn nog niet wat ze moeten zijn ;-)

ElsLommelen commented 3 years ago

Hierbij een extra poging, voor een vorige poging zie commit a56aa07. En om eerlijk te zijn, de tussenliggende pogingen heb ik niet bewaard: getest, en als test niet bleek te werken, gewoon iets anders getest zonder te committen. En intussen weet ik al niet meer wat ik allemaal getest heb, ik vermoed verschillende pogingen om een figuur toe te voegen aan die yml (op basis van voorbeelden die ik op het internet gevonden had).

ElsLommelen commented 3 years ago

@leymanan Zoals eerder voorgesteld, heb ik een link toegevoegd naar de poster die ik op ISEC presenteerde, en nog wat logo's (twitter, facebook, en het logo van ANB bij copyrightholder, al gaat dat laatste logo enkel zichtbaar zijn als de website online gebouwd wordt, dus niet als je het bouwt met pkgdown::build_site()). De meest ingrijpende aanpassing t.o.v. je vorige check is dus het tekstje van de poster.

Geef jij nog even je goedkeuring om te mergen? Opgelet, deze keer niet zelf mergen, omwille van het toevoegen van de automatische tests ga ik dat als admin zelf moeten doen!

(Ik zie net dat de check faalt, dit los ik uiteraard nog op voor het mergen.)

ElsLommelen commented 3 years ago

Ok, probleem is alvast opgelost. :-)

ElsLommelen commented 3 years ago

Voorgaande code lost issue #50 op, en enkele andere problemen i.v.m. continuous integration van de github actions (fixes #50)

leymanan commented 2 years ago

Els, ik heb nu problemen met de validatierapporten. Bij elk type krijg ik dezelfde, bijna lege output: bevat enkel Validatierapport Els Lommelen 2022-01-10 cat(Uitvoer, sep = "\n")

Kan dat te maken hebben met een Ik heb al eens debug laten lopen bij de validatie-functie:

debug(validatie.basis)
temp <- validatie.basis(Basismodel, TypeRapport = "Statisch")

Alles lijkt te kloppen, maar validatierapport is leeg?

MAAR: op 6/1 is het me wel nog gelukt?? Raar ... Daarom denk ik dat het te maken heeft met geheugenruimte? Kan dat? Ik dacht even aan de setup van mijn chunks, maar ook als ik de code direct kleef in het commando-venster heb ik zelfde resultaat? Maar ik ga dat nog eens verder onderzoeken ...

leymanan commented 2 years ago

ik ben nu bezig enkel op lokale modellen te focussen, om zo geheugenruimte niet teveel te belasten, maar op eerste zicht maakt dat niet veel uit. Nu nog eens opnieuw, na herstarten R.

leymanan commented 2 years ago

als ik een heel beknopte code run in een R-script (geen Rmd), lukt het wel.

leymanan commented 2 years ago

Lukt me ook in mijn Rmd-script, als ik enkel de code mbv lokaal model laat lopen. Ik denk dat dat toch iets te maken heeft met geheugenruimte.

leymanan commented 2 years ago

Problemen hadden inderdaad te maken met te weinig geheugenruimte: als die onder de 800 MiB blijft lukt het wel.

ElsLommelen commented 2 years ago

Met de info die je geeft, heb ik eerlijk gezegd geen idee wat evt. het probleem zou kunnen zijn. Krijg je een foutmelding, en zo ja, welke? En welke output zit er in temp? Welke gegevens heb je gebruikt? Kan het zijn dat die gegevens al eerder gevalideerd zijn en dat er dus geen problemen meer in zitten? Of heb je in je dataset enkel de gegevens van de lokale modellen gestoken, en probeer je voor deze gegevens een basismodel te bouwen? Als er in de opgegeven dataset geen problemen meer zitten die je moet valideren, is het perfect normaal dat je een leeg validatierapport krijgt, dus dit kan ook gewoon goed nieuws zijn.

leymanan commented 2 years ago

Opgelost, zie github!

Op ma 10 jan. 2022 om 13:48 schreef ElsLommelen @.***>:

Met de info die je geeft, heb ik eerlijk gezegd geen idee wat evt. het probleem zou kunnen zijn. Krijg je een foutmelding, en zo ja, welke? En welke output zit er in temp? Welke gegevens heb je gebruikt? Kan het zijn dat die gegevens al eerder gevalideerd zijn en dat er dus geen problemen meer in zitten? Of heb je in je dataset enkel de gegevens van de lokale modellen gestoken, en probeer je voor deze gegevens een basismodel te bouwen? Als er in de opgegeven dataset geen problemen meer zitten die je moet valideren, is het perfect normaal dat je een leeg validatierapport krijgt, dus dit kan ook gewoon goed nieuws zijn.

— Reply to this email directly, view it on GitHub https://github.com/inbo/dhcurve/pull/44#issuecomment-1008842735, or unsubscribe https://github.com/notifications/unsubscribe-auth/AGKXN5BQ6472MRFWJLSW6I3UVLITHANCNFSM4U3CVLYQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because you were mentioned.Message ID: @.***>

--

Anja Leyman

Expert Cel Beheerplanning en Monitoring

//////////////////////////////////////////////////////////////////////////////////////////////////////////////////////

Vlaamse overheid

AGENTSCHAP NATUUR & BOS

Standplaats Instituut voor Natuur- en Bosonderzoek (INBO) Gaverstraat 4, 9500 Geraardsbergen T: +32 495 14 90 60 E-mail: @. @.>*

www.natuurenbos.be http://www.natuurenbos.be/

De inhoud van dit bericht en eventuele bijlage(n) verbinden het Agentschap voor Natuur en Bos niet, zolang niet bevestigd door een geldig ondertekend document

ElsLommelen commented 2 years ago

@leymanan Ik heb het ANB-logo toegevoegd op de navbar, zoals besproken in #52, maar de inhoudsopgave rechts bijwerken lukt blijkbaar niet (zie hier). Ik kijk nog even hoe ik die error in wercker best omzeil, en dan merge ik deze alvast?

leymanan commented 2 years ago

Bedankt!

Ik had dat vorige week ook al gezien, en de site nog eens gebouwd mbv "pkgdown::build_site()" Ik zag toe en ook nu dat handleiding.html en index.html aangepast wordt (obv de datum), maar als ik de html's open, zie ik logo van ANB niet verschijnen?

[image: image.png]

Ik krijg wel volgende warning:

Warning message:The vignette title specified in \VignetteIndexEntry{} is different from the title in the YAML metadata. The former is "Handleiding dhcurve", and the latter is "Handleiding package diameter-hoogtecurves (dhcurve)". If that is intentional, you may set options(rmarkdown.html_vignette.check_title = FALSE) to suppress this check.

Op di 18 jan. 2022 om 15:07 schreef ElsLommelen @.***>:

@leymanan https://github.com/leymanan Ik heb het ANB-logo toegevoegd op de navbar, zoals besproken in #52 https://github.com/inbo/dhcurve/issues/52, maar de inhoudsopgave rechts bijwerken lukt blijkbaar niet (zie hier https://github.com/r-lib/pkgdown/issues/1357). Ik kijk nog even hoe ik die error in wercker best omzeil, en dan merge ik deze alvast?

— Reply to this email directly, view it on GitHub https://github.com/inbo/dhcurve/pull/44#issuecomment-1015444771, or unsubscribe https://github.com/notifications/unsubscribe-auth/AGKXN5BC5GYVPM4J6JESB4TUWVXY3ANCNFSM4U3CVLYQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because you were mentioned.Message ID: @.***>

--

Anja Leyman

Expert Cel Beheerplanning en Monitoring

//////////////////////////////////////////////////////////////////////////////////////////////////////////////////////

Vlaamse overheid

AGENTSCHAP NATUUR & BOS

Standplaats Instituut voor Natuur- en Bosonderzoek (INBO) Gaverstraat 4, 9500 Geraardsbergen T: +32 495 14 90 60 E-mail: @. @.>*

www.natuurenbos.be http://www.natuurenbos.be/

De inhoud van dit bericht en eventuele bijlage(n) verbinden het Agentschap voor Natuur en Bos niet, zolang niet bevestigd door een geldig ondertekend document

ElsLommelen commented 2 years ago

Klopt, ik werk met een link, waardoor die figuur pas zichtbaar wordt nadat ze online staat. Als je met je muis links van de facebook- en twitteraccount beweegt, zie je door de verandering van je muispointer wel al de achterliggende link naar jullie website. Maar ik zal het meteen even mergen, dan kunnen we testen of het online wel in orde is.