reconhub / incidence

☣:chart_with_upwards_trend::chart_with_downwards_trend:☣ Compute and visualise incidence
https://reconhub.github.io/incidence
Other
58 stars 13 forks source link

Patch issue 119 (broken plots) #120

Closed thibautjombart closed 4 years ago

thibautjombart commented 4 years ago

This is a patch for plot.incidence which is currently broken due to this issue in ggplot2, as described in https://github.com/reconhub/incidence/issues/119.

Outline of patch

This fixes the issue on:

> R.version
               _                           
platform       x86_64-pc-linux-gnu         
arch           x86_64                      
os             linux-gnu                   
system         x86_64, linux-gnu           
status                                     
major          3                           
minor          6.3                         
year           2020                        
month          02                          
day            29                          
svn rev        77875                       
language       R                           
version.string R version 3.6.3 (2020-02-29)
nickname       Holding the Windsock        

Unrelated stuff that came up

I get unrelated warnings re documentation (see below) which I ignored as probably unrelated to this patch.

❯ checking Rd files ... WARNING
  prepare_Rd: man/fit.Rd:36: unknown macro '\item'
  prepare_Rd: man/fit.Rd:40: unknown macro '\item'
  prepare_Rd: man/fit.Rd:43: unknown macro '\item'
  prepare_Rd: man/fit.Rd:46: unknown macro '\item'
  prepare_Rd: man/fit.Rd:52: unknown macro '\item'
  prepare_Rd: man/fit.Rd:54: unexpected section header '\value'
  prepare_Rd: man/fit.Rd:94: unexpected section header '\description'
  prepare_Rd: man/fit.Rd:107: unexpected section header '\examples'
  prepare_Rd: man/fit.Rd:152: unexpected section header '\seealso'
  prepare_Rd: man/fit.Rd:157: unexpected section header '\author'
  prepare_Rd: man/fit.Rd:161: unexpected END_OF_INPUT '
  '
  checkRd: (5) fit.Rd:0-162: Must have a \description

❯ checking Rd \usage sections ... WARNING
  Undocumented arguments in documentation object 'fit'
    ‘quiet’ ‘window’ ‘plot’ ‘separate_split’ ‘...’

  Functions with \usage entries need to have the appropriate \alias
  entries, and all their arguments documented.
  The \usage entries must correspond to syntactically valid R code.
  See chapter ‘Writing R documentation files’ in the ‘Writing R
  Extensions’ manual.
zkamvar commented 4 years ago

Thank you for jumping on this quickly, Teebz :)

It does fix the problem stated, but creates the problem that the model fitting no longer works fits curves on the edges of the bars instead of the middle. That being said, I don't think I would have come up with anything different given the ggplot2 bug.

A side note: there is also a new fun issue from R 4.0's default of stringsAsFactors = FALSE: https://travis-ci.org/reconhub/incidence/jobs/660131096#L8198-L8202

This popped up because we never enabled cron jobs on travis for this repo, but live and learn.

thibautjombart commented 4 years ago

I have recompiled the doc with R 3.6.3 and devtools_2.2.2 but did not find any immediate fix for the warnings on fit.Rd. I cannot reproduce the issues reported on Travis.

zkamvar commented 4 years ago

I cannot reproduce the issues reported on Travis.

This is because it only exists on R-devel. The solution is to explicitly state stringsAsFactors = TRUE wherever data frames are being created (likely in the tests files).

thibautjombart commented 4 years ago

I have not chased the source of the warning, working flat out on covid19 work. Will call for help on RECON slack to finish the PR. More and more people are reporting this issue.

dirkschumacher commented 4 years ago

@zkamvar @thibautjombart seems that fixed the isses in the tests