"quiet_require" and "import_r" with default parameter "suppress_messages=True" don't work

Issue #119 resolved
Pierre-Louis Bonicoli created an issue

Hi,

It seems the quiet_require function and "import_r" with default parameter are unusable.

from rpy2.robjects.packages import quiet_require, importr
import rpy2.robjects as robjects
r_path = robjects.r['file.path']
importr("Cairo", r_path("/home/pilou/.R/x86_64-pc-linux-gnu-library/2.15"))

Output:

Traceback (most recent call last):
  File "/tmp/test.py", line 10, in <module>
    r_path("/home/pilou/.R/x86_64-pc-linux-gnu-library/2.15"))
  File "/usr/lib/python2.7/dist-packages/rpy2/robjects/packages.py", line 270, in importr
    ok = quiet_require(name, lib_loc = lib_loc)
  File "/usr/lib/python2.7/dist-packages/rpy2/robjects/packages.py", line 47, in quiet_require
    expr = rinterface.parse(expr_txt)
ValueError: Error while parsing the string.

If i modify the default parameter "suppress_messages" of "import", it works:

from rpy2.robjects.packages import quiet_require, importr
import rpy2.robjects as robjects
r_path = robjects.r['file.path']
importr("Cairo", r_path("/home/pilou/.R/x86_64-pc-linux-gnu-library/2.15"), suppress_messages=False)

Output:

Loading required package: Cairo
  1. It seems that quotes are missing in quiet_require function,

    suppressPackageStartupMessages(base::require(Cairo, lib.loc=/home/pilou/.R/x86_64-pc-linux-gnu-library/2.15))
    

    is an invalid R syntax and

    suppressPackageStartupMessages(base::require("Cairo", lib.loc="/home/pilou/.R/x86_64-pc-linux-gnu-library/2.15"))
    

    is OK.

  2. When suppress_messages is True, lib_loc parameter must not be a string or unicode and file.path must be used but when suppress_messages is False, lib_loc could be a string or unicode.

I propose to replace quiet_require function with require function with quietly=True parameter.

Comments (9)

  1. Laurent Gautier

    Thanks.

    Point 1: Double quotes are indeed missing (and so is a unit-test that would have prevented the problem to remain unnoticed). I also see that the coding style is hackish (pre-dates convenience features that appear later in rpy2) - quiet_require should probably be rewritten.

    Point 2: I am not following this completely. Once the issue with quote is fixed, this should just work.

  2. Laurent Gautier

    I have just committed a minimal change that fixes the problem (and more importantly added a unit test).

    Your pull request has the merit of simplifying the code base of rpy2, but I had problems with the require() parameter quietly to really be quiet. Are you really really sure that it is the case (the package "Cairo" does not generate any output, so quiet require() or not there will not be any difference - try "randomForest" or "Biobase" in bioconductor).

  3. Pierre-Louis Bonicoli reporter

    About point 2 see attached file: "unit_test_1.patch".

    Adding quote isn't a right solution, see attached file "unit_test_2.patch". To my mind, the best way to fix the problem is to avoid call to eval. Indeed suppress_messages=True suppress all messages (not only PackageStartupMessages) A best solution could be to remove quiet_require method (this method never works).

    About commit 2b39608, why don't you follow pep8 ?

  4. Laurent Gautier

    ...because no one expects the PEP inquisition. That's a long PEP, any hint about the specific sins I committed ?

    The claim that quiet_require never works is raising suspicion about the other claims ;-)

    Here I can run without any error (rpy2 - branch version_2.3.x ):

    from rpy2.robjects.packages import quiet_require
    
    quiet_require("stats")
    quiet_require("Biobase")
    
    
    from rpy2.robjects.packages import get_packagepath
    stats = quiet_require("stats", get_packagepath('stats'))
    
  5. Pierre-Louis Bonicoli reporter

    Indeed quiet_require was working but only when lib_loc parameter was not used (fixed by commit 2b39608), so completely removing it would be drastic ;)

    About pep8, you could use flake8.

    Thanks!

  6. Log in to comment