Henrik Bengtsson
2017-Mar-03 18:10 UTC
[Rd] Control statements with condition with greater than one should give error (not just warning) [PATCH]
On Fri, Mar 3, 2017 at 9:55 AM, Hadley Wickham <h.wickham at gmail.com> wrote:>> But, how you propose a warning-to-error transition should be made >> without wreaking havoc? Just flip the switch in R-devel and see CRAN >> and Bioconductor packages break overnight? Particularly Bioconductor >> devel might become non-functional (since at times it requires >> R-devel). For my own code / packages, I would be able to handle such >> a change, but I'm completely out of control if one of the package I'm >> depending on does not provide a quick fix (with the only option to >> remove package tests for those dependencies). > > Generally, a package can not be on CRAN if it has any warnings, so I > don't think this change would have any impact on CRAN packages. Isn't > this also true for bioconductor?Having a tests/warn.R file with: warning("boom") passes through R CMD check --as-cran unnoticed. Same with: if (sample(2) == 1) message("It's your lucky day today!") /Henrik PS. Does testthat signal that?> > Hadley > > -- > http://hadley.nz
Martin Maechler
2017-Mar-04 20:04 UTC
[Rd] Control statements with condition with greater than one should give error (not just warning) [PATCH]
>>>>> Henrik Bengtsson <henrik.bengtsson at gmail.com> >>>>> on Fri, 3 Mar 2017 10:10:53 -0800 writes:> On Fri, Mar 3, 2017 at 9:55 AM, Hadley Wickham > <h.wickham at gmail.com> wrote: >>> But, how you propose a warning-to-error transition >>> should be made without wreaking havoc? Just flip the >>> switch in R-devel and see CRAN and Bioconductor packages >>> break overnight? Particularly Bioconductor devel might >>> become non-functional (since at times it requires >>> R-devel). For my own code / packages, I would be able >>> to handle such a change, but I'm completely out of >>> control if one of the package I'm depending on does not >>> provide a quick fix (with the only option to remove >>> package tests for those dependencies). >> >> Generally, a package can not be on CRAN if it has any >> warnings, so I don't think this change would have any >> impact on CRAN packages. Isn't this also true for >> bioconductor? > Having a tests/warn.R file with: > warning("boom") > passes through R CMD check --as-cran unnoticed. Yes, indeed.. you are right Henrik that many/most R warning()s would not produce R CMD check 'WARNING's .. I think Hadley and I fell into the same mental pit of concluding that such warning()s from if(<length-larger-one>) ... would not currently happen in CRAN / Bioc packages and hence turning them to errors would not have a direct effect. With your 2nd e-mail of saying that you'd propose such an option only for a few releases of R you've indeed clarified your intent to me. OTOH, I would prefer using an environment variable (as you've proposed as an alternative) which is turned "active" at the beginning only manually or for the "CRAN incoming" checks of the CRAN team (and bioconductor submission checks?) and later for '--as-cran' etc until it eventually becomes the unconditional behavior of R (and the env.variable is no longer used). Martin
Michael Lawrence
2017-Mar-04 20:20 UTC
[Rd] Control statements with condition with greater than one should give error (not just warning) [PATCH]
Is there really a need for these complications? Packages emitting this warning are broken by definition and should be fixed. Perhaps we could "flip the switch" in a test environment and see how much havoc is wreaked and whether authors are sufficiently responsive? Michael On Sat, Mar 4, 2017 at 12:04 PM, Martin Maechler <maechler at stat.math.ethz.ch> wrote:> >>>>> Henrik Bengtsson <henrik.bengtsson at gmail.com> > >>>>> on Fri, 3 Mar 2017 10:10:53 -0800 writes: > > > On Fri, Mar 3, 2017 at 9:55 AM, Hadley Wickham > > <h.wickham at gmail.com> wrote: > >>> But, how you propose a warning-to-error transition > >>> should be made without wreaking havoc? Just flip the > >>> switch in R-devel and see CRAN and Bioconductor packages > >>> break overnight? Particularly Bioconductor devel might > >>> become non-functional (since at times it requires > >>> R-devel). For my own code / packages, I would be able > >>> to handle such a change, but I'm completely out of > >>> control if one of the package I'm depending on does not > >>> provide a quick fix (with the only option to remove > >>> package tests for those dependencies). > >> > >> Generally, a package can not be on CRAN if it has any > >> warnings, so I don't think this change would have any > >> impact on CRAN packages. Isn't this also true for > >> bioconductor? > > > Having a tests/warn.R file with: > > > warning("boom") > > > passes through R CMD check --as-cran unnoticed. > > Yes, indeed.. you are right Henrik that many/most R warning()s would > not produce R CMD check 'WARNING's .. > > I think Hadley and I fell into the same mental pit of concluding > that such warning()s from if(<length-larger-one>) ... > would not currently happen in CRAN / Bioc packages and hence > turning them to errors would not have a direct effect. > > With your 2nd e-mail of saying that you'd propose such an option > only for a few releases of R you've indeed clarified your intent > to me. > OTOH, I would prefer using an environment variable (as you've > proposed as an alternative) which is turned "active" at the > beginning only manually or for the "CRAN incoming" checks of > the CRAN team (and bioconductor submission checks?) > and later for '--as-cran' etc until it eventually becomes the > unconditional behavior of R (and the env.variable is no longer used). > > Martin > > ______________________________________________ > R-devel at r-project.org mailing list > https://stat.ethz.ch/mailman/listinfo/r-devel >[[alternative HTML version deleted]]
Reasonably Related Threads
- Control statements with condition with greater than one should give error (not just warning) [PATCH]
- Control statements with condition with greater than one should give error (not just warning) [PATCH]
- Control statements with condition with greater than one should give error (not just warning) [PATCH]
- Control statements with condition with greater than one should give error (not just warning) [PATCH]
- Control statements with condition with greater than one should give error (not just warning) [PATCH]