From ddf1c9ec8194191a844742e2fcd626b5022558d4 Mon Sep 17 00:00:00 2001
From: Michaela Mueller <51025211+mumichae@users.noreply.github.com>
Date: Tue, 22 Nov 2022 23:16:18 +0100
Subject: [PATCH] Fix kBET import issue (#350)

* use r-lib actions

* move r-lib to top

* add R dependency install

* return error instead of np.nan when kBET is not installed

* install devtools via R

* use rp2y

* use remotes instead of devtools

* move r-lib actions to test workflow

* add GITHUB_PAT

* separate run for R functions

* use correct R version

* install test dependencies
---
 .github/workflows/test.yml      | 43 ++++++++++++++++++++++++++++-----
 README.md                       |  7 ++++++
 scib/exceptions.py              |  6 +++--
 scib/metrics/kbet.py            |  4 +--
 tests/metrics/rpy2/test_kbet.py |  6 ++---
 5 files changed, 52 insertions(+), 14 deletions(-)

diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
index 4854fb8d..2e60aff7 100644
--- a/.github/workflows/test.yml
+++ b/.github/workflows/test.yml
@@ -28,7 +28,7 @@ jobs:
             run: |
                 pre-commit run --color=always --show-diff-on-failure --all-files
 
-    test:
+    metrics:
         runs-on: ${{ matrix.os }}
         strategy:
             matrix:
@@ -64,19 +64,50 @@ jobs:
                     --ignore=tests/integration/ \
                     --ignore=tests/metrics/rpy2 -vv
 
-        -   name: Test R metrics
-            if: ${{ matrix.os != 'macos-latest'}}
+        -   name: Upload coverage
+            if: ${{ matrix.os != 'macos-latest' }}
+            env:
+                CODECOV_NAME: ${{ matrix.os }}-${{ matrix.python }}
             run: |
-                pip install '.[rpy2]'
-                python -m pytest --cov --cov-append --cov-report=term-missing -k rpy2 -vv
+                codecov --no-color --required --flags unittest
 
+    rpy2:
+        runs-on: ${{ matrix.os }}
+        strategy:
+            matrix:
+                r: [4.2]
+                python: [3.9]
+                os: [ubuntu-latest]
+        steps:
+        -   uses: actions/checkout@v2
+        -   name: Set up Python ${{ matrix.python }}
+            uses: actions/setup-python@v2
+            with:
+                python-version: ${{ matrix.python }}
+        -   name: Set up R
+            uses: r-lib/actions/setup-r@v2
+            with:
+                r-version: ${{ matrix.r }}
+
+        -   name: Install dependencies
+            env:
+                GITHUB_PAT: ${{ secrets.GITHUB_TOKEN }}
+            run: |
+                python -m pip install --upgrade pip
+                pip install '.[test,rpy2]'
+                Rscript -e "install.packages('remotes')"
+                Rscript -e "remotes::install_github('theislab/kBET')"
+
+        -   name: Test with pytest
+            run: |
+                python -m pytest --cov --cov-append --cov-report=term-missing -k rpy2 -vv
         -   name: Upload coverage
-            if: ${{ matrix.os != 'macos-latest' }}
             env:
                 CODECOV_NAME: ${{ matrix.os }}-${{ matrix.python }}
             run: |
                 codecov --no-color --required --flags unittest
 
+
     integration:
         runs-on: ${{ matrix.os }}
         strategy:
diff --git a/README.md b/README.md
index 14cf08bd..fce68c34 100644
--- a/README.md
+++ b/README.md
@@ -68,6 +68,13 @@ e.g. for installing `rpy2` and `bbknn` dependencies:
 pip install 'scib[rpy2,bbknn]'
 ```
 
+Optional dependencies outside of python need to be installed separately.
+For instance, in order to run kBET, install it via the following command in R:
+
+```R
+remotes::install_github('theislab/kBET')
+```
+
 ## Metrics
 
 We implemented different metrics for evaluating batch correction and biological conservation in the `scib.metrics`
diff --git a/scib/exceptions.py b/scib/exceptions.py
index b72d7b2e..70e5e83c 100644
--- a/scib/exceptions.py
+++ b/scib/exceptions.py
@@ -1,7 +1,9 @@
 class OptionalDependencyNotInstalled(ModuleNotFoundError):
-    def __init__(self, exception):
+    def __init__(self, exception, module_name=None):
+        if module_name is None:
+            module_name = exception.name
         self.message = (
-            f"\n{exception.name} is an optional dependency and not installed by default. "
+            f"\n'{module_name}' is an optional dependency and not installed by default. "
             f"Please make sure you install it manually."
         )
         super().__init__(self.message)
diff --git a/scib/metrics/kbet.py b/scib/metrics/kbet.py
index 418940b3..33e460b4 100644
--- a/scib/metrics/kbet.py
+++ b/scib/metrics/kbet.py
@@ -52,8 +52,8 @@ def kBET(
 
     try:
         ro.r("library(kBET)")
-    except rpy2.rinterface_lib.embedded.RRuntimeError:
-        return np.nan
+    except rpy2.rinterface_lib.embedded.RRuntimeError as e:
+        raise OptionalDependencyNotInstalled(e, "kBET")
 
     # compute connectivities for non-knn type data integrations
     # and increase neighborhoods for knn type data integrations
diff --git a/tests/metrics/rpy2/test_kbet.py b/tests/metrics/rpy2/test_kbet.py
index 5f5520a5..c12add32 100644
--- a/tests/metrics/rpy2/test_kbet.py
+++ b/tests/metrics/rpy2/test_kbet.py
@@ -1,7 +1,5 @@
-import numpy as np
-
 import scib
-from tests.common import LOGGER
+from tests.common import LOGGER, assert_near_exact
 
 
 def test_kbet(adata_pca):
@@ -9,4 +7,4 @@ def test_kbet(adata_pca):
         adata_pca, batch_key="batch", label_key="celltype", embed="X_pca"
     )
     LOGGER.info(f"score: {score}")
-    assert np.isnan(score)
+    assert_near_exact(score, 0.556108994805538, diff=1e-02)