Helseatlas / helseatlas

Shiny-app to replace the current IA
MIT License
3 stars 2 forks source link

Modul - draft #142

Closed Tovjoh closed 4 years ago

Tovjoh commented 4 years ago

Jeg har gjort et forsøk på å lage en modul for fanene med kart, søylediagram og tabell. Fanene kommer fram med ikonene sine, men figurene er fraværende.

arnfinn commented 4 years ago

Bra jobba! Skal jeg se litt på det? Mulig modulen ikke kjenner til verdiene i input og at disse burde sendes inn til funksjonen tab_server. Noe ala dette:

tab_server <- function(id, filtered_data) {

Her har jeg sendt inn filtered_data, slik at man slipper å regne ut dette hver gang man bytter fane. Mulig andre verdier også må sendes inn, slik som config.

Tovjoh commented 4 years ago

Du må gjerne se på det :)

arnfinn commented 4 years ago

Jeg har prøvd meg litt frem, men kommer ikke så langt... Tenker kanskje vi må tenke tre moduler:

Prøver meg litt frem i #143

Tovjoh commented 4 years ago

Det virker fornuftig å ha koden som lager kartet inni kart-modulen, det gjør det mer oversiktlig. Jeg skal se mer på det i morgen. Har lagd en tom app med to moduler som jeg skal "bygge" på, for å bedre se hvordan de ulike elementene henger sammen. @arnfinn

arnfinn commented 4 years ago

Jeg tenkte noe slik i app_ui.r:

shiny::tabPanel(shiny::icon("globe"), map_ui("map")),
shiny::tabPanel(shiny::icon("chart-bar"), histogram_ui("hist")),
shiny::tabPanel(shiny::icon("table"), table_ui("table"))

Jeg prøvde meg litt på map_ui/map_server, men fikk det ikke helt til. Mulig jeg prøver meg på noe enklere først... Disse drop-down-menyene f.eks. Prøver litt videre i morgen jeg også.

arnfinn commented 4 years ago

Mulig jeg tar feil og at det er bedre å gjøre det slik du har tenkt her, etter å ha tenkt meg om litt... Vi vil jo etterhvert at disse tre "modulene" (kart, histogram, tabell) skal kunne markeres etc. på tvers av hverandre. At man kan velge et HF på kart, som så også vil være markert når man går inn i tabell. Mulig dette blir vanskelig hvis de deles inn i tre ulike shiny moduler.

Tovjoh commented 4 years ago

Eller...hva om vi har flere ui`er men en server "to rule them all"? @arnfinn :)

UI_modul:

panel_ui1 <- function(id) {
  ns <- shiny::NS(id)
    tagList(leaflet::leafletOutput(ns("plot_map"), height = 800))}

panel_ui2 <- function(id) {
  ns <- shiny::NS(id)
    tagList(shiny::plotOutput(ns("plot_histogram"), height = 800))} 

Server_modul:

panel_server <- function(id) {
  shiny::moduleServer(id, function(input, output, session) {
output$plot_map <- leaflet::renderLeaflet({  })
output$plot_histogram <- shiny::renderPlot({  })
}

App_ui:

shiny::tabsetPanel(
                                      shiny::tabPanel(shiny::icon("globe"), panel_ui1("Map")),
                                      shiny::tabPanel(shiny::icon("chart-bar"), panel_ui2("Map"))

App_server:

app_server <- function(input, output, session) {
panel_server("Map")
}
Tovjoh commented 4 years ago

Fordelen slik jeg ser det, er at de valgene som tas ifbm atlas og tema, de blir sendt til alle tre fanene. Det neste blir å få sidevalgmenyen til å fungere fra modul. Tror jeg unner meg en kopp kaffe først!

arnfinn commented 4 years ago

Din plan høres fornuftig ut. Jeg kan prøve å se på sidevalgmeny.

En kaffe skal du få lov til å ta først:)

arnfinn commented 4 years ago

Tenker at vi legger inn #145 først og så denne? Så kan vi sende inn filtrert data inn i denne modulen.

arnfinn commented 4 years ago

@Tovjoh Jeg kan prøve å merge master inn i denne branchen. Er sikkert en del konflikter...

Tovjoh commented 4 years ago

@arnfinn vent litt med å gjøre det er du snill. Var ikke meningen å trykke på merge på fredagen..

Jeg gjør et forsøk nå på å få select modulen til å kommunisere med tabs modulen.

arnfinn commented 4 years ago

Men klarer du å merge master inn i denne branchen? Jeg pushet akkurat en endring der jeg kun tilbakestilte en del mellomrom i app_server.r og app_ui.r, slik at det blir likt slik det var på master. Ellers tror git at det er gjort endringer i hver eneste linje i app_server i denne branchen og vi får dermed konflikter på hver eneste linje som er endret i master. Hvis du skjønner hva jeg mener...

Tovjoh commented 4 years ago

Det er bare å kansellere den. Var ikke helt meningen å trykke merge som sagt.

arnfinn commented 4 years ago

Det er bare å kansellere den. Var ikke helt meningen å trykke merge som sagt.

Usikker på hva du mener... Har du trykket merge noen plass?

Tovjoh commented 4 years ago

Det er bare å kansellere den. Var ikke helt meningen å trykke merge som sagt.

Usikker på hva du mener... Har du trykket merge noen plass?

Ja på fredagen.. merge master into modul.

arnfinn commented 4 years ago

Det er bare å kansellere den. Var ikke helt meningen å trykke merge som sagt.

Usikker på hva du mener... Har du trykket merge noen plass?

Ja på fredagen.. merge master into modul.

Det går ikke så lenge det er konflikter. Og det er det nå. Men for at du skal kunne fortsette her så må master inn i denne branchen. Og det kan jeg prøve å fikse, hvis det er OK for deg?

EDIT: Jeg har puttet master inn i denne branchen lokalt og er klar til å trykke push...

Tovjoh commented 4 years ago

Det er bare å kansellere den. Var ikke helt meningen å trykke merge som sagt.

Usikker på hva du mener... Har du trykket merge noen plass?

Ja på fredagen.. merge master into modul.

Det går ikke så lenge det er konflikter. Og det er det nå. Men for at du skal kunne fortsette her så må master inn i denne branchen. Og det kan jeg prøve å fikse, hvis det er OK for deg?

Hvis master skal inn her så fiks i vei. Men hvis det krever mye arbeid så er det kanskje lettere at jeg lager meg en ny branch fra master? Du bestemmer.

Tovjoh commented 4 years ago

Det er bare å kansellere den. Var ikke helt meningen å trykke merge som sagt.

Usikker på hva du mener... Har du trykket merge noen plass?

Ja på fredagen.. merge master into modul.

Det går ikke så lenge det er konflikter. Og det er det nå. Men for at du skal kunne fortsette her så må master inn i denne branchen. Og det kan jeg prøve å fikse, hvis det er OK for deg?

EDIT: Jeg har puttet master inn i denne branchen lokalt og er klar til å trykke push...

Do it!

arnfinn commented 4 years ago

@Tovjoh Skal jeg se litt på det?

Bare legg merke til det som står her: https://mastering-shiny.org/index.html :)

This is the online version of Mastering Shiny, a book currently under early development and intended for a late 2020 release by O’Reilly Media.

Men jeg tror det er fornuftig å lese i denne "boka", spesielt siden den er "up-to-date".

Tovjoh commented 4 years ago

@Tovjoh Skal jeg se litt på det?

Bare legg merke til det som står her: https://mastering-shiny.org/index.html :)

This is the online version of Mastering Shiny, a book currently under early development and intended for a late 2020 release by O’Reilly Media.

Men jeg tror det er fornuftig å lese i denne "boka", spesielt siden den er "up-to-date".

Du trenger ikke min tillatelse til å se på det. Slå deg løs! :)

arnfinn commented 4 years ago

Både map og data må være reactive.

Nå ble sikker alt mye klarere:)

arnfinn commented 4 years ago

@Tovjoh Kan du sjekke nå om alt ser OK ut?

Det kommer fremdeles feilmelding mens appen lastes inn, men ikke så mange som tidligere. Det som gjenstår er å

Tovjoh commented 4 years ago

Både map og data må være reactive.

  • Hvis map ikke er reactive så vil ikke denne oppdateres hvis man bytter atlas. healthatlas_data[[selection$atlas()]]$map er ikke reactive, så bakte denne inn i shiny::reactive() når den sendes inn i modulen.
  • data() er ikke reactive, men en verdi. Ved å fjerne () så sender man inn en (reactive) funksjon i stedet for en verdi.

Nå ble sikker alt mye klarere:)

Krystall!! Eller...mulig litt grums enda. Skal se når jeg har fått lest litt. Jeg måtte en tur innom kvalitetsatlas-prosjektet, men resten av dagen skal jeg bruke i R!

Tovjoh commented 4 years ago

@Tovjoh Kan du sjekke nå om alt ser OK ut?

Det kommer fremdeles feilmelding mens appen lastes inn, men ikke så mange som tidligere. Det som gjenstår er å

  • "linte" pakken. Kjøre lintr::lint_package() og fikse de advarslene som kommer ut. Du må gjerne også bruke kortere innrykk i module_tabs.r. To mellomrom pr. nivå?
  • å lage tester av modulen. Du kan se i filen tests/testthat/test_module_select.R for å se hvordan det er gjort for module_select og prøve å gjøre noe tilsvarende for module_tabs.

@arnfinn Jeg har begynt å lage test til modulen. Hvor detaljerte skal testene være? Teste data som kommer inn, at det har den formen vi forventer. Deretter at plot genereres uten feilmelding. Annet?

arnfinn commented 4 years ago

@Tovjoh Ja, noe ala det. Litt begrensinger på hvor nøye vi kan teste det som kommer ut, siden det kan være ganske avanserte objekter.

Jeg pleier å legge inn noe ala dette

expect_equal_to_reference(output$plot_histogram, "test.rds")

i testen og så åpne test.rds i Rstudio og undersøke denne

test <- readRDS("tests/testthat/test.rds")
View(test)

(kan også gjøres med litt klikk og pek).

Trykk så på denne f.eks:

Skjermbilde 2020-05-13 kl  17 07 59

som er det samme som

test[["html"]]

Du kan også se på hvordan jeg har gjort det her (f.eks linje 11 og 12): https://github.com/mong/qmongr/blob/master/tests/testthat/test-mod_quality_overview.R Jeg gjør output$treatment_unit[["html"]] om til characters og søker i denne.

        treatment_units <- as.character(output$treatment_unit[["html"]])
        expect_true(grepl("Hammerfest", treatment_units))
Tovjoh commented 4 years ago

@arnfinn Hvis jeg ikke har misforstått helt så er det de samme argumentene som går inn i modul-server som vi skriver som argumenter i testdelen? Needless to say, den fungerer ikke som den skal! Jeg så nærmere på output fra plot_histogram. Det kommer ut i src, ikke html, og det er ikke mulig å søke etter navn slik du har gjort i andre tester.

arnfinn commented 4 years ago

@Tovjoh Jeg prøver litt jeg også. Ett par ting jeg har gjort foreløpig:

filtered_data <- readRDS("data/filtered_data_4.rds")
kart <- kart::dagkir2

.
.
.

args = list(data = shiny::reactive(filtered_data),
...

Men jeg får det fremdeles ikke helt til her heller...

Tovjoh commented 4 years ago

@Tovjoh Jeg prøver litt jeg også. Ett par ting jeg har gjort foreløpig:

filtered_data <- readRDS("data/filtered_data_4.rds")
kart <- kart::dagkir2

.
.
.

args = list(data = shiny::reactive(filtered_data),
...
  • filtered_data_3 var noe gammelt, som ikke hadde tall i area-kolonnen.
  • Bruker kart fra kart-pakken, siden vi vet disse er ok. Eventuelt kan vi lagre dette kartet som rds-fil, slik at vi ikke er avhengig av at det ikke skjer endringer i kart-pakken.
  • data må være reactive.

Men jeg får det fremdeles ikke helt til her heller...

Kartet har jeg hentet fra kart-pakken, men jeg lagret det ikke som rds-fil. Jeg brukte sf::st_write() for å lagre kartet som shp-fil.

Jeg skal se mer på det litt senere. Først skal jeg lese i Mastering Shiny, også driver jeg og leker meg med purrr.

arnfinn commented 4 years ago

OK. Jeg vil helst unngå å bruke shp-filer... Gammel og utdatert teknologi.

arnfinn commented 4 years ago

Ser ut til at jeg må fikse litt på datafilene i tests/testthat/data/-mappen. Av en eller annen grunn har jeg satt area lik area_name og ikke numerisk i atlas-dataene. Ser også at area_name mangler i enkelte datasett.

arnfinn commented 4 years ago

Testen ser nå slik ut:

  # filtrert data
  filtered_data <- readRDS("data/filter_2.rds")
  kart <- kart::dagkir

  shiny::testServer(tab_server, {

    expect_equal(class(output$plot_map), "json")
    map <- as.character(output$plot_map)
    # Check that it is some human made values in map
    expect_true(grepl("<strong>Forde", map))
    # Check that map is a large json...
    expect_true(nchar(map) > 1000000)

    expect_equal(class(output$make_table), "json")
    table <- as.character(output$make_table)
    # Check that it is some human made values in table
    expect_true(grepl("<th>Innbyggere", table))

    histogram <- output$plot_histogram
    # Check y values (name of health trusts)
    expect_equal_to_reference(histogram[["coordmap"]][["panels"]][[1]][["domain"]][["discrete_limits"]][["y"]], "data/plot_histogram_y.rds")
    expect_equal(histogram[["coordmap"]][["panels"]][[1]][["mapping"]][["x"]], "get(\"value\")")
    expect_equal(histogram[["coordmap"]][["panels"]][[1]][["mapping"]][["y"]], "get(\"area_name\")")
    expect_equal(histogram[["coordmap"]][["panels"]][[1]][["mapping"]][["fill"]], "get(\"brks\")")

  }, args = list(data = shiny::reactive(filtered_data),
                  map = shiny::reactive(kart),
                  config = readRDS("data/get_config.rds"),
                  language = shiny::reactive("nb")
                  )
  )

Jeg sjekker ikke output direkte mot referanseverdier, siden de inneholder en del informasjon som vil variere fra maskin til maskin (vil jeg tro). Dessuten var output$plot_map ganske stor.