Repository: simpeg/simpeg
Pull request #1514 (2024-08-08 07:53:52)

Make `pandas` & `scikit-learn` optional dependencies
#### Summary

Set `sklearn` and `pandas` as optional dependencies in `pyproject.toml`. Make use of `discretize`'s `requires` utils function to decorate the functions and methods that require either `pandas` or `scikit-learn` to be installed, specifically in `simpeg.electromagnetics.static.resisitivity.IO` and PGI-related classes.

`sklearn` and `pandas` are comparably heavy dependency, but they are only used in one specific place each:

- `pandas`: `simpeg/electromagnetics/static/resistivity/IODC.py`;
- `sklearn`: `simpeg/utils/pgi_utils.py`.

As such, they are potentially only used by a fraction of the SimPEG users.

This PR implements them as soft dependencies, in the same way as `matplotlib` is a soft dependency in `discretize`.

#### PR Checklist
* [x] Linted my code according to the [style guides](https://docs.simpeg.xyz/content/getting_started/contributing/code-style.html).
* [ ] Added [tests](https://docs.simpeg.xyz/content/getting_started/practices.html#testing) to verify changes to the code.
* [ ] Added necessary documentation to any new functions/classes following the
      expect [style](https://docs.simpeg.xyz/content/getting_started/practices.html#documentation).
* [x] Marked as ready for review (if this is was a draft PR), and converted 
      to a Pull Request
* [x] Tagged ``@simpeg/simpeg-developers`` when ready for review.

#### What does this implement/fix?
An installation of SimPEG will not install `sklearn` nor `pandas`. If a function/class is called that requires one of these packages, the following message is given:
```
Missing dependencies: ['sklearn'].
Not running `__init__`.
```

If we want to make this message more explicit (e.g., with instructions how to install it), we'll have to adjust the `requires` function in `discretize`.

#### Comments
- I did not add any tests - I guess the functionality of `requires` is tested in `discretize`. However, I can add some if wanted.
- Not sure if it should be mentioned at the appropriate places in the documentation.

codecov[bot]: ## [Codecov](https://app.codecov.io/gh/simpeg/simpeg/pull/1514?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=simpeg) Report
Attention: Patch coverage is `71.42857%` with `8 lines` in your changes missing coverage. Please review.
> Project coverage is 50.13%. Comparing base [(`c699a84`)](https://app.codecov.io/gh/simpeg/simpeg/commit/c699a84f699699f94ef2066288268e13c6090913?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=simpeg) to head [(`a1c87b4`)](https://app.codecov.io/gh/simpeg/simpeg/commit/a1c87b4f0cf1280637f7cfcd759b1cb01377485c?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=simpeg).

| [Files](https://app.codecov.io/gh/simpeg/simpeg/pull/1514?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=simpeg) | Patch % | Lines |
|---|---|---|
| [simpeg/electromagnetics/static/resistivity/IODC.py](https://app.codecov.io/gh/simpeg/simpeg/pull/1514?src=pr&el=tree&filepath=simpeg%2Felectromagnetics%2Fstatic%2Fresistivity%2FIODC.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=simpeg#diff-c2ltcGVnL2VsZWN0cm9tYWduZXRpY3Mvc3RhdGljL3Jlc2lzdGl2aXR5L0lPREMucHk=) | 54.54% | [5 Missing :warning: ](https://app.codecov.io/gh/simpeg/simpeg/pull/1514?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=simpeg) |
| [simpeg/utils/pgi\_utils.py](https://app.codecov.io/gh/simpeg/simpeg/pull/1514?src=pr&el=tree&filepath=simpeg%2Futils%2Fpgi_utils.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=simpeg#diff-c2ltcGVnL3V0aWxzL3BnaV91dGlscy5weQ==) | 82.35% | [3 Missing :warning: ](https://app.codecov.io/gh/simpeg/simpeg/pull/1514?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=simpeg) |

> :exclamation:  There is a different number of reports uploaded between BASE (c699a84) and HEAD (a1c87b4). Click for more details.
> 
> <details><summary>HEAD has 26 uploads less than BASE</summary>
>
>| Flag | BASE (c699a84) | HEAD (a1c87b4) |
>|------|------|------|
>||30|4|
></details>

<details><summary>Additional details and impacted files</summary>


```diff
@@             Coverage Diff             @@
##             main    #1514       +/-   ##
===========================================
- Coverage   86.24%   50.13%   -36.12%     
===========================================
  Files         396      309       -87     
  Lines       50329    41582     -8747     
  Branches     5308     4939      -369     
===========================================
- Hits        43405    20846    -22559     
- Misses       5473    19905    +14432     
+ Partials     1451      831      -620     
```

</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/simpeg/simpeg/pull/1514?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=simpeg).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=simpeg).

santisoler: Thanks @prisae for opening this. I think the PR is in good shape to be merged.

One comment I have is that it would be better if the `requires` decorator throws an `ImportError` or `ModuleNotFoundError` if the optional dependency is missing. I think that could improve the user experience. For example, let's say you have a preexisting code that was saving the `IO` as a csv with `write_to_csv`. If in your new environment you forgot to install `pandas`, then the same code will run without failing, although the csv file won't be created and the user will only get one printed message in the stdout. This comment is outside the scope of this PR, but maybe this is something we could add to the decorator in discretize. What do you think?

One caveat of having errors being raised, is that we should test if every module can be imported without having optional dependencies installed. Is not a big deal, but I would like to have that automatically running before the conda-forge tests run a similar one after a release.
prisae: One reason against throwing an error is that errors frighten the un-experienced user, and they are often not able to properly read them (in my experience). This is why I opted for an informative message instead of an error. But surely something that can be discussed.
santisoler: Makes sense! I'm merging this now. We can keep discussing that in discretize. Thanks again for this @prisae!

Patch:
diff --git a/environment.yml b/environment.yml
index 5fdf11c81..6d07ad0fc 100644
--- a/environment.yml
+++ b/environment.yml
@@ -6,13 +6,11 @@ dependencies:
   - python=3.11
   - numpy>=1.20
   - scipy>=1.8
-  - scikit-learn>=1.2
   - pymatsolver-base>=0.2
   - matplotlib-base
   - discretize>=0.10
   - geoana>=0.5.0
   - empymod>=2.0.0
-  - pandas
 
 # solver
 # uncomment the next line if you are on an intel platform
@@ -25,6 +23,8 @@ dependencies:
   - choclo
   - scooby
   - plotly
+  - scikit-learn>=1.2
+  - pandas
 
 # documentation building
   - sphinx
diff --git a/pyproject.toml b/pyproject.toml
index 6a8926907..476e59b18 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -17,13 +17,11 @@ keywords = [
 dependencies = [
     "numpy>=1.20",
     "scipy>=1.8",
-    "scikit-learn>=1.2",
     "pymatsolver>=0.2",
     "matplotlib",
     "discretize>=0.10",
     "geoana>=0.5.0",
     "empymod>=2.0.0",
-    "pandas",
 ]
 classifiers = [
     "Development Status :: 4 - Beta",
@@ -55,8 +53,10 @@ dask = ["dask", "zarr", "fsspec>=0.3.3"]
 choclo = ["choclo"]
 reporting = ["scooby"]
 plotting = ["plotly"]
+sklearn = ["scikit-learn>=1.2"]
+pandas = ["pandas"]
 all = [
-    "simpeg[dask,choclo,plotting,reporting]"
+    "simpeg[dask,choclo,plotting,reporting,sklearn,pandas]"
 ] # all optional *runtime* dependencies (not related to development)
 style = [
     "black==24.3.0",
diff --git a/simpeg/electromagnetics/static/resistivity/IODC.py b/simpeg/electromagnetics/static/resistivity/IODC.py
index cc47403d7..912bccc7a 100644
--- a/simpeg/electromagnetics/static/resistivity/IODC.py
+++ b/simpeg/electromagnetics/static/resistivity/IODC.py
@@ -1,5 +1,4 @@
 import numpy as np
-import pandas as pd
 import matplotlib.pyplot as plt
 import matplotlib
 import warnings
@@ -7,6 +6,12 @@ import warnings
 from discretize import TensorMesh, TreeMesh
 from discretize.base import BaseMesh
 from discretize.utils import refine_tree_xyz, unpack_widths, active_from_xyz
+from discretize.utils import requires as module_requires
+
+try:
+    import pandas
+except ImportError:
+    pandas = False
 
 from ....utils import (
     sdiag,
@@ -1283,6 +1288,7 @@ class IO:
         survey.topo = topo
         return survey
 
+    @module_requires({"pandas": pandas})
     def write_to_csv(self, fname, dobs, standard_deviation=None, **kwargs):
         uncert = kwargs.pop("uncertainty", None)
         if uncert is not None:
@@ -1300,7 +1306,7 @@ class IO:
             dobs,
             standard_deviation,
         ]
-        df = pd.DataFrame(
+        df = pandas.DataFrame(
             data=data,
             columns=[
                 "Ax",
@@ -1317,8 +1323,9 @@ class IO:
         )
         df.to_csv(fname)
 
+    @module_requires({"pandas": pandas})
     def read_dc_data_csv(self, fname, dim=2):
-        df = pd.read_csv(fname)
+        df = pandas.read_csv(fname)
         if dim == 2:
             a_locations = df[["Ax", "Az"]].values
             b_locations = df[["Bx", "Bz"]].values
@@ -1352,8 +1359,9 @@ class IO:
             raise NotImplementedError()
         return survey
 
+    @module_requires({"pandas": pandas})
     def read_topo_csv(self, fname, dim=2):
         if dim == 2:
-            df = pd.read_csv(fname)
+            df = pandas.read_csv(fname)
             topo = df[["X", "Z"]].values
         return topo
diff --git a/simpeg/utils/code_utils.py b/simpeg/utils/code_utils.py
index 8d7fd2046..691865d13 100644
--- a/simpeg/utils/code_utils.py
+++ b/simpeg/utils/code_utils.py
@@ -478,11 +478,9 @@ class Report(ScoobyReport):
             "pymatsolver",
             "numpy",
             "scipy",
-            "sklearn",
             "matplotlib",
             "empymod",
             "geoana",
-            "pandas",
         ]
 
         # Optional packages.
@@ -491,6 +489,8 @@ class Report(ScoobyReport):
             "pydiso",
             "numba",
             "dask",
+            "sklearn",
+            "pandas",
             "sympy",
             "IPython",
             "ipywidgets",
diff --git a/simpeg/utils/pgi_utils.py b/simpeg/utils/pgi_utils.py
index 196957de2..145850f03 100644
--- a/simpeg/utils/pgi_utils.py
+++ b/simpeg/utils/pgi_utils.py
@@ -3,24 +3,34 @@ import matplotlib.pyplot as plt
 from mpl_toolkits.axes_grid1.inset_locator import inset_axes
 from scipy import linalg
 from scipy.special import logsumexp
-from sklearn.mixture import GaussianMixture
-from sklearn.cluster import KMeans
-from sklearn.utils import check_array
-from sklearn.utils.validation import check_is_fitted
-from sklearn.mixture._gaussian_mixture import (
-    _compute_precision_cholesky,
-    _compute_log_det_cholesky,
-    _estimate_gaussian_covariances_full,
-    _estimate_gaussian_covariances_diag,
-    _estimate_gaussian_covariances_spherical,
-    _check_means,
-    _check_precisions,
-    _check_shape,
-)
-from sklearn.mixture._base import check_random_state, ConvergenceWarning
 import warnings
 from simpeg.maps import IdentityMap
 
+from discretize.utils.code_utils import requires
+
+# sklearn is a soft dependency
+try:
+    import sklearn
+    from sklearn.mixture import GaussianMixture
+    from sklearn.cluster import KMeans
+    from sklearn.utils import check_array
+    from sklearn.utils.validation import check_is_fitted
+    from sklearn.mixture._gaussian_mixture import (
+        _compute_precision_cholesky,
+        _compute_log_det_cholesky,
+        _estimate_gaussian_covariances_full,
+        _estimate_gaussian_covariances_diag,
+        _estimate_gaussian_covariances_spherical,
+        _check_means,
+        _check_precisions,
+        _check_shape,
+    )
+    from sklearn.mixture._base import check_random_state, ConvergenceWarning
+
+except ImportError:
+    GaussianMixture = None
+    sklearn = False
+
 
 ###############################################################################
 # Disclaimer: the following classes built upon the GaussianMixture class      #
@@ -31,7 +41,7 @@ from simpeg.maps import IdentityMap
 ###############################################################################
 
 
-class WeightedGaussianMixture(GaussianMixture):
+class WeightedGaussianMixture(GaussianMixture if sklearn else object):
     """
     Weighted Gaussian mixture class
 
@@ -65,6 +75,7 @@ class WeightedGaussianMixture(GaussianMixture):
         Active indexes
     """
 
+    @requires({"sklearn": sklearn})
     def __init__(
         self,
         n_components,
@@ -841,6 +852,7 @@ class GaussianMixtureWithPrior(WeightedGaussianMixture):
         Shape is (index of the fixed cell, lithology index) fixed_membership:
     """
 
+    @requires({"sklearn": sklearn})
     def __init__(
         self,
         gmmref,
@@ -1236,6 +1248,7 @@ class GaussianMixtureWithNonlinearRelationships(WeightedGaussianMixture):
         List of mapping describing a nonlinear relationships between physical properties; one per cluster/unit.
     """
 
+    @requires({"sklearn": sklearn})
     def __init__(
         self,
         mesh,
@@ -1546,6 +1559,7 @@ class GaussianMixtureWithNonlinearRelationshipsWithPrior(GaussianMixtureWithPrio
 
     """
 
+    @requires({"sklearn": sklearn})
     def __init__(
         self,
         gmmref,


Test patch:
diff --git a/.ci/environment_test.yml b/.ci/environment_test.yml
index 500304509..1b947eb46 100644
--- a/.ci/environment_test.yml
+++ b/.ci/environment_test.yml
@@ -4,13 +4,11 @@ channels:
 dependencies:
   - numpy>=1.20
   - scipy>=1.8
-  - scikit-learn>=1.2
   - pymatsolver-base>=0.2
   - matplotlib-base
   - discretize>=0.10
   - geoana>=0.5.0
   - empymod>=2.0.0
-  - pandas
 
 # Solver
   - pydiso
@@ -22,6 +20,8 @@ dependencies:
   - choclo
   - scooby
   - plotly
+  - scikit-learn>=1.2
+  - pandas
 
 # documentation building
   - sphinx
diff --git a/tests/utils/test_report.py b/tests/utils/test_report.py
index 828d06ec3..bd6238712 100644
--- a/tests/utils/test_report.py
+++ b/tests/utils/test_report.py
@@ -15,11 +15,9 @@ class TestReport(unittest.TestCase):
                 "pymatsolver",
                 "numpy",
                 "scipy",
-                "sklearn",
                 "matplotlib",
                 "empymod",
                 "geoana",
-                "pandas",
             ],
             # Optional packages.
             optional=[
@@ -27,6 +25,8 @@ class TestReport(unittest.TestCase):
                 "pydiso",
                 "numba",
                 "dask",
+                "sklearn",
+                "pandas",
                 "sympy",
                 "IPython",
                 "ipywidgets",