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

Afwerking package #44

Merged
merged 142 commits into from
Jan 18, 2022
Merged

Afwerking package #44

merged 142 commits into from
Jan 18, 2022

Conversation

ElsLommelen
Copy link
Collaborator

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

  • Checks: als deze op punt staan (= aangepast zijn aan het package), stel ik voor om deze te gebruiken in plaats van de huidige checks van Wercker, die het tegenwoordig nogal eens laten afweten wegens 'dagelijks quotum opgebruikt'.
  • code of conduct, contributing guidelines, news,... Deze items zijn automatisch mee toegevoegd, ik heb de contributing guidelines aangepast waar strikt nodig en de rest gelaten als is (in het Engels).
  • website: voorlopig ook gewoon overgenomen, dit is hetzelfde stijl als die van het package checklist zelf. (Lokaal te bekijken met pkgdown::build_site().) Vermits het copyright bij ANB staat en niet bij INBO, mss beter dat we er een ANB-stijl van proberen te maken? Hiervoor zijn er 2 gemakkelijke opties:
    • als je me een css-bestand kan bezorgen van de website van ANB, dan kunnen we eens testen wat het geeft als we die gebruiken
    • we kunnen de huidige stijl behouden en dan de kleuren vervangen door die van ANB (al eens beginnen met fuchsia te vervangen door groen, ik weet niet of jullie ook andere specifieke kleuren gebruiken?). Ik weet niet of je evt. aan de RGB-codes van die kleuren kan geraken (om de exacte kleur te kunnen gebruiken)?
  • Ah, en bezorg je voor de website evt. ook de facebook- en twitter-links van ANB, of moet er dat niet op? (Voor mij een kleintje om dat even aan te passen, als de urls zelf niet moet zoeken. ;-) ) En als je andere zaken ziet die voor ANB anders moeten zijn, laat zeker weten.

(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 ElsLommelen marked this pull request as draft December 14, 2020 17:36
@ElsLommelen
Copy link
Collaborator Author

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.

# Conflicts:
#	DESCRIPTION
#	man/fit.afgeleid.Rd
#	man/fit.basis.Rd
#	man/fit.lokaal.Rd
#	man/initiatie.Rd
#	man/leesFile.Rd
#	man/validatie.afgeleid.Rd
#	man/validatie.basis.Rd
#	man/validatie.lokaal.Rd
(aangepast naar nieuwe default in functie)
@ElsLommelen
Copy link
Collaborator Author

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

@leymanan
Copy link
Collaborator

leymanan commented Jan 10, 2022

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
Copy link
Collaborator

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
Copy link
Collaborator

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

@leymanan
Copy link
Collaborator

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
Copy link
Collaborator

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

@ElsLommelen
Copy link
Collaborator Author

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
Copy link
Collaborator

leymanan commented Jan 10, 2022 via email

@ElsLommelen
Copy link
Collaborator Author

@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
Copy link
Collaborator

leymanan commented Jan 18, 2022 via email

@ElsLommelen
Copy link
Collaborator Author

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.

@ElsLommelen ElsLommelen merged commit 9ab518c into develop Jan 18, 2022
@ElsLommelen ElsLommelen deleted the afwerking branch January 18, 2022 15:44
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.

4 participants