search

United4Surveillance / signal-detection-tool

A tool for detection of signals in infectious disease surveillance data.
Other
8 stars 0 forks source link

removing restrictions for the stratified timeseries plot in the report #297

Closed tinneuro closed 3 months ago

tinneuro commented 3 months ago

@alchalu Why did you decide to introduce the limit for the facet_wrap in the report? :) I understand that with a lot of strata it might be overwhelming but currently looks ok. I was confused that the stratified figures for the age groups were not shown anymore in the report, that is how I found out about it.

What do you think about removing this statement like done in this PR or increasing the limit to 25 for now? If we decide to not show the stratified timeseries because too many strata we should maybe

alchalu commented 3 months ago

Your right! Not informing the user makes this choice confusing. When creating the HTML report we do not need a limit. In the word report we can only show 12 strata per page. I will try making a version with ggplot2::facet_wrap_paginate().

alchalu commented 3 months ago

Oh... facet_wrap_paginate() is a ggforce function 😞

tinneuro commented 3 months ago

ah but at least I understood the problem now. :) Well if facet_wrap_paginate() is an easy solution let's introduce it even though it has the additional dependency. Otherwise we find another solution or we make a quick first solution that in the html you can see all strata and in the word you get an info about it.

tinneuro commented 3 months ago

Hi, are you going to add the facet_wrap_paginate() or should I?

tinneuro commented 3 months ago

I tested it and it looks good. :) A minor addition for the future which we can turn into a new issue is that the age group facets are sorted using the ordering through a factor if it is there. But I would merge this now. Can you approve the review, so we can merge? :)