United4Surveillance / signal-detection-tool

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

update: make run_report a standalone function #325

Closed alchalu closed 6 days ago

alchalu commented 2 months ago

Makes it possible to run report as a standalone function with only data as an input

tinneuro commented 2 months ago

Hi @alchalu ,

great work thanks for that! I tested it and everything is working for me. While testing I had two minor remarks:

As always could you also open up a new section in NEWS.md and write that you implemented the standalone report function?

Thanks!

tinneuro commented 1 month ago

Hi @alchalu ,

do you think you can incooperate the changes this week?

alchalu commented 1 month ago

Hi @tinneuro,

thank you for your comments!

tinneuro commented 1 month ago

When I try to run the report using the following settings: run_report(input_example, method = "glm mean") I get an error message and it says that it quitted from lines 50-52 in the SignalDetectionReport.Rmd and Missing value, where TRUE/FALSE is needed. Same for the other glm methods. Can you investigate this?

tinneuro commented 1 month ago

It is great that the user now has the opportunity to specify an output_dir. Can you also add this to the NEWS that this is possible now?

tinneuro commented 3 weeks ago

Hi @alchalu,

can you manage to work on this this week? Thanks!

alchalu commented 3 weeks ago

Done. Sorry for the delay!

tinneuro commented 2 weeks ago

Hi :) Thanks for the updates, unfortunately I still run into a bug. When I try to execute run_report() just like this it does not work for me. Maybe it is some problem with my packages? Does it work for you? It says Quitting from lines 50-52 [unnamed-chunk-2] (SignalDetectionReport.Rmd)

NorbertHandraAGES commented 2 weeks ago

Hey @tinneuro,

I believe there was a problem with missing namespaces of non-exported functions in the report markdown. You also need to reinstall the package from this branch since the markdown files used for rendering are taken from system.file(), i.e. your library. Could you check if this fixed your issue?

tinneuro commented 1 week ago

Thanks @NorbertHandraAGES ! I finally managed to get it running. I also reinstalled the package from this branch before but got the error nevertheless. Now I still had a different but weird error that the png() device is not working and after some googling found out that this is a Windows problem when the Rproject lies in a folder with a long path this can happen. So I changed the location to a shorter path and it worked. https://stackoverflow.com/questions/45113597/error-unable-to-start-png-device I will finally review the rest tomorrow.

tinneuro commented 1 week ago

Hi :) I found one more issue. The run_report() function works well. But when I use the app and click on the "Create Report" Button the report is generated and also already saved automatically under a automatically generated name i.e. file3b744c796c38.docx but in the menu which then pops up where the user can actually select where to save the report "report-downloadReport.htm" this shows up instead of the report. Can we make it again that the correct .html or .docx shows up there and that it is named SignalDetectionReport and not saved beforehand with some automatic generated filename?

tinneuro commented 1 week ago

Furthermore when using the function I realised that the users of the app are used to the algorithm names they see in the app i.e. FarringtonFlexible instead of "farrington" we use in the background and so on. Thus I decided to change the code such that these method names we use in the app are also used in the run_report function. I will commit this now. Maybe you can review this? I will also add some tests in my next commit.

alchalu commented 1 week ago

Thank you so much for the changes and the test! They all look good to me.

alchalu commented 1 week ago

I just pushed a quick fix for the download directory. Now the default download directory is used when downloading from the app and the current working directory is used when running run_report() directly from an R session.

alchalu commented 1 week ago

I think the branch can finally be merged. @tinneuro can you do this? I can't merge my own request and Norbert is on vaccation. Thank you so much!

tinneuro commented 6 days ago

Great thanks for the fix! It looks all good now.