Feature/add age as filter variable

United4Surveillance / signal-detection-tool

A tool for detection of signals in infectious disease surveillance data.

Other

8 stars 0 forks source link

Feature/add age as filter variable #324

Open aj-thl opened 3 weeks ago

aj-thl commented 3 weeks ago

Adding age as a possible filter variable.

tinneuro commented 1 week ago

Thanks a lot for adding this feature! I tried it out and it works. Some minor comments:

It is possible for me to select a min age > max age which then gives an error for the signal detection algorithms etc. The same that I can select a max age < min age. Can you somehow restrict this still so that it is not possible?
Stylewise: for date report we put the box for min date report and max date report next to each other, currently for min and max age they are below each other. Do you think you can also put them next to each other?
Can you update your branch with the latest develop?
Can you then add what you did to a new NEWS section?

aj-thl commented 1 week ago

Thanks @tinneuro!

Thanks for pointing out the inconsistency regarding the selection of min and max age. I will fix this. Similar situation occurs if user selects both age and age group as a filtering variables. It is possible for the user to make conflicting choices. For example, one can filter age < 50 and age group 60-69. I don't know if we want to fix this in the code or leave it to the user to check if the filtering makes sense. For age alone (force selection of min and max age consistent in the code) it should be easy to fix but if we need to adjust filtering for the age groups as well, it gets complicated.

Also, I will try to put the min and max age next to each other.

I always forget the NEWS sorry about that... will add these changes to it.

tinneuro commented 1 week ago

Great, thanks you also realised that. If you cant fix the issue with conflicting filters in age and age group we can make a new feature out of this. I think in the beginning users will not really do that so it is fine if it is too much work to fix it in this PR. And no worries you forgot about NEWS ;)

aj-thl commented 1 week ago

I see if I can do something about the age groups as well. I think I already solved the other issues :) I try to finish this by the end of this week.

tinneuro commented 6 days ago

Great, let me know when you are done.

aj-thl commented 4 days ago

Ok, so I think I'm finished with this one @tinneuro. I was not able to get the filtering with both age and age group work logically. The solution should simply be to set filter_choices for age group according to the filtered ages in the mod_input_filter.R but no matter how hard I tried I could not get the filtering to work.

I would propose that we make a separate issue about inconsistent filtering options with age and age group and and someone looks at it with fresh eyes.

tinneuro commented 1 day ago

Great. Thanks for trying! Yes let's do this. Can you create an issue for this?

aj-thl commented 1 day ago

Sure! I will create an issue about this.