Maximize code coverage in test suite

.coveragerc

@@ -0,0 +1,11 @@
+# replace by exclude_also after bumping to >=7.2
+# with that, "pragma: no cover" can be removed
+[report]
+exclude_lines =
+    def __str__
+    def __repr__
+    pragma: no cover
+
+[run]
+omit =
+  invisible_cities/database/*

.github/workflows/coverage.yml

@@ -0,0 +1,43 @@
+# This workflow will install IC and run all the tests with pytest
+
+name: Test suite
+
+on:
+  pull_request:
+
+jobs:
+  check-coverage:
+    runs-on: ubuntu-latest
+
+    steps:
+    - uses: actions/checkout@v4
+    - name: Get LFS files
+      run: git lfs pull
+
+    - name: Install IC
+      run: |
+        source $CONDA/etc/profile.d/conda.sh
+        source manage.sh work_in_python_version_no_tests 3.8
+
+    - name: Produce coverage report
+      run: |
+        source $CONDA/etc/profile.d/conda.sh
+        source manage.sh work_in_python_version_no_tests 3.8
+        coverage run -m pytest
+        coverage report -m > coverage_report
+
+    - name: Check coverage report
+      run: |
+        echo "Showing only files with coverage < 100%"
+        head -2 coverage_report
+        incomplete_coverage=$(head -n -2 coverage_report | tail -n +3 | grep -v "100%")
+
+        while read line; do
+          printf '\033[0;31m%s\033[0m\n' "$line" # red color ... reset color
+        done <<<${incomplete_coverage}
+
+        tail -2 coverage_report
+
+        if [ ! -z "$incomplete_coverage" ]; then
+          echo "::warning ::Test suite coverage incomplete"
+        fi

.github/workflows/test_suite.yml

@@ -7,7 +7,7 @@ on:
   pull_request:
 
 jobs:
-  build:
+  build-and-test:
 
     strategy:
       fail-fast: false
@@ -30,7 +30,6 @@ jobs:
         source $CONDA/etc/profile.d/conda.sh
         source manage.sh work_in_python_version_no_tests ${{ matrix.python-version }}
 
-
     - name: Run tests
       run: |
         set -o pipefail # necessary for test failure to propagate to `tee`

invisible_cities/calib/calib_functions_test.py

@@ -15,6 +15,7 @@
 from .. core                 import   tbl_functions as tbl
 from .. core                 import   fit_functions as fitf
 from .. core                 import system_of_units as units
+from .. core.core_functions  import        in_range
 from .. core.stat_functions  import   poisson_sigma
 from .. evm.nh5              import     SensorTable
 from .. types.symbols        import      SensorType
@@ -148,9 +149,9 @@ def test_copy_sensor_table(config_tmpdir, sensor_type, sensors):
 @mark.parametrize('sensor_type     , n_channel, gain_seed, gain_sigma_seed',
                   ((SensorType.SIPM,         1,   16.5622,         2.5),
                    (SensorType.PMT ,         5,   24.9557,         9.55162)))
-def test_seeds_db(sensor_type, n_channel, gain_seed, gain_sigma_seed, dbnew):
+def test_seeds_db(sensor_type, n_channel, gain_seed, gain_sigma_seed):
     run_number = 6217
-    result = cf.seeds_db(sensor_type, dbnew, run_number, n_channel)
+    result = cf.seeds_db(sensor_type, "new", run_number, n_channel)
     assert result == (gain_seed, gain_sigma_seed)
 
 
@@ -190,14 +191,16 @@ def test_sensor_values(sensor_type, scaler, expected_range, min_b, max_b, half_w
 @mark.parametrize('sensor_type, run_number, n_chann,            scaler',
                   ((      None,       6217,    1023, _dark_scaler_sipm),
                    (      None,       6217,       0, _dark_scaler_pmt)))
-def test_incorrect_sensor_type_raises_ValueError(sensor_type, dbnew, run_number, n_chann, scaler):
+def test_incorrect_sensor_type_raises_ValueError(sensor_type, run_number, n_chann, scaler):
     bins     = np.array([ -6,  -5,   -4,   -3,   -2,   -1,    0,    1,    2,    3,    4,   5,   6,   7])
     spec     = np.array([ 28, 539, 1072, 1845, 2805, 3251, 3626, 3532, 3097, 2172, 1299, 665, 371, 174])
     ped_vals = np.array([2.65181178e+04, 1.23743445e-01, 2.63794236e+00])
 
     with raises(ValueError):
-        cf.       seeds_db(sensor_type, dbnew, run_number, n_chann)
+        cf.       seeds_db(sensor_type, "new", run_number, n_chann)
+    with raises(ValueError):
         cf.poisson_mu_seed(sensor_type, scaler, bins, spec, ped_vals)
+    with raises(ValueError):
         cf.  sensor_values(sensor_type, scaler, bins, spec, ped_vals)
 
 
@@ -214,6 +217,7 @@ def test_pedestal_values():
     assert_approx_equal(ped_values.sigma_min,      0.001)
 
 
+@mark.slow
 def test_compute_seeds_from_spectrum(ICDATADIR):
     PATH_IN = os.path.join(ICDATADIR, 'sipmcalspectra_R6358.h5')
     # Suppress warnings from division by zero in some bins.
@@ -227,12 +231,10 @@ def test_compute_seeds_from_spectrum(ICDATADIR):
         for ich, (led, dar) in enumerate(zip(specsL, specsD)):
             b1 = 0
             b2 = len(dar)
-            try:
-                valid_bins = np.argwhere(led>=min_stat)
-                b1 = valid_bins[ 0][0]
-                b2 = valid_bins[-1][0]
-            except IndexError:
-                continue
+
+            valid_bins = np.argwhere(led>=min_stat)
+            b1 = valid_bins[ 0][0]
+            b2 = valid_bins[-1][0]
 
             peaks_dark = find_peaks_cwt(dar, np.arange(2, 20), min_snr=2)
             if len(peaks_dark) == 0:
@@ -260,7 +262,53 @@ def test_compute_seeds_from_spectrum(ICDATADIR):
             assert gain_sigma_seed != 0
 
 
-def test_seeds_without_using_db(ICDATADIR, dbnew):
+@mark.filterwarnings("ignore:Covariance of the parameters could not be estimated")
+def test_seeds_and_bounds_with_db():
+    values        = np.array([1, 0.5, .1], dtype=float)
+    errors        = (values + 1) / 20
+    x             = np.arange(-50, 300)
+    y             = fitf.gauss(x, *values)
+    sel           = in_range(x, -5, 5, right_closed=True) # magic
+    scaler_func   = cf.dark_scaler(y[sel])
+    seeds, bounds = cf.seeds_and_bounds(SensorType.SIPM, 8000, 0,
+                                        scaler_func, x, y,
+                                        values, "new", errors,
+                                        use_db_gain_seeds=True)
+
+
+    # this test doesn't do much, to be improved
+    assert all(seeds)
+    assert len(seeds)     == 4
+    assert len(bounds)    == 2
+    assert len(bounds[0]) == 4
+    assert len(bounds[1]) == 4
+
+
+@mark.filterwarnings("ignore:Covariance of the parameters could not be estimated")
+def test_seeds_and_bounds_with_gau():
+    values        = np.array([1, 0.5, .1], dtype=float)
+    errors        = (values + 1) / 20
+    x             = np.arange(-50, 300)
+    y             = fitf.gauss(x, *values)
+    sel           = in_range(x, -5, 5, right_closed=True) # magic
+    scaler_func   = cf.dark_scaler(y[sel])
+    seeds, bounds = cf.seeds_and_bounds(SensorType.SIPM, 8000, 0,
+                                        scaler_func, x, y,
+                                        values, "new", errors,
+                                        "gau",
+                                        use_db_gain_seeds=True)
+
+
+    # this test doesn't do much, to be improved
+    assert all(seeds)
+    assert len(seeds)     == 6
+    assert len(bounds)    == 2
+    assert len(bounds[0]) == 6
+    assert len(bounds[1]) == 6
+
+
+@mark.slow
+def test_seeds_without_using_db(ICDATADIR):
     PATH_IN = os.path.join(ICDATADIR, 'sipmcalspectra_R6358.h5')
     # Suppress warnings from division by zero in some bins.
     with warnings.catch_warnings(), tb.open_file(PATH_IN) as h5in:
@@ -275,12 +323,10 @@ def test_seeds_without_using_db(ICDATADIR, dbnew):
         for ich, (led, dar) in enumerate(zip(specsL, specsD)):
             b1 = 0
             b2 = len(dar)
-            try:
-                valid_bins = np.argwhere(led>=min_stat)
-                b1 = valid_bins[ 0][0]
-                b2 = valid_bins[-1][0]
-            except IndexError:
-                continue
+
+            valid_bins = np.argwhere(led>=min_stat)
+            b1 = valid_bins[ 0][0]
+            b2 = valid_bins[-1][0]
 
             peaks_dark = find_peaks_cwt(dar, np.arange(2, 20), min_snr=2)
             if len(peaks_dark) == 0:
@@ -300,8 +346,23 @@ def test_seeds_without_using_db(ICDATADIR, dbnew):
             scaler_func   = cf.dark_scaler(dar[p_range][p_bins])
             seeds, bounds = cf.seeds_and_bounds(SensorType.SIPM, run_no, ich,
                                                 scaler_func, bins[p_range],
-                                                led[p_range], ped_vals, dbnew,
+                                                led[p_range], ped_vals, "new",
                                                 gfitRes.errors,
                                                 use_db_gain_seeds=False)
             assert all(seeds)
             assert bounds == ((0, 0, 0, 0.001), (np.inf, 10000, 10000, 10000))
+
+
+@mark.parametrize( "f nargs".split()
+                 , ( (cf.       seeds_db, 3)
+                   , (cf.poisson_mu_seed, 4)
+                   , (cf.  sensor_values, 4)))
+def test_sensor_type_input(f, nargs):
+    """
+    Test that if the first argument (sensor_type) is not a symbol, the
+    functions complain. The rest of the arguments are irrelevant, so
+    they are just dummy values.
+    """
+    dummy = [None]*nargs
+    with raises(ValueError): f( "PMT", *dummy)
+    with raises(ValueError): f("SiPM", *dummy)

invisible_cities/calib/calib_sensors_functions.py

@@ -10,10 +10,6 @@
 from ..types.symbols import SiPMCalibMode
 
 
-def mask_sensors(wfs, active):
-    return wfs * active.astype(wfs.dtype).reshape(active.size, 1)
-
-
 def scipy_mode(x, axis=0):
     """
     Scipy implementation of the mode (runs very slow).

invisible_cities/cities/beersheba.py

@@ -187,15 +187,6 @@ def deconvolve_signal(det_db           : pd.DataFrame,
                                **satellite_params,
                                inter_method = inter_method)
 
-    if not isinstance(energy_type , HitEnergy          ):
-        raise ValueError(f'energy_type {energy_type} is not a valid energy type.')
-    if not isinstance(inter_method, InterpolationMethod):
-        raise ValueError(f'inter_method {inter_method} is not a valid interpolation method.')
-    if not isinstance(cut_type    , CutType            ):
-        raise ValueError(f'cut_type {cut_type} is not a valid cut type.')
-    if not isinstance(deconv_mode , DeconvolutionMode  ):
-        raise ValueError(f'deconv_mode {deconv_mode} is not a valid deconvolution mode.')
-
     def deconvolve_hits(df, z):
         '''
         Given an slice, applies deconvolution using the PSF
@@ -259,7 +250,14 @@ def apply_deconvolution(df):
     return apply_deconvolution
 
 
-def create_deconvolution_df(hits, deconv_e, pos, cut_type, e_cut, n_dim):
+@check_annotations
+def create_deconvolution_df( hits     : pd.DataFrame
+                           , deconv_e : np.ndarray
+                           , pos      : Tuple[np.ndarray, np.ndarray, np.ndarray]
+                           , cut_type : CutType
+                           , e_cut    : float
+                           , n_dim    : int
+                           ) -> pd.DataFrame:
     '''
     Given the output of the deconvolution, it cuts the low energy voxels and
     creates a dataframe object with the resulting output.
@@ -280,12 +278,8 @@ def create_deconvolution_df(hits, deconv_e, pos, cut_type, e_cut, n_dim):
 
     df  = pd.DataFrame(columns=['event', 'npeak', 'X', 'Y', 'Z', 'E'])
 
-    if   cut_type is CutType.abs:
-        sel_deconv = deconv_e > e_cut
-    elif cut_type is CutType.rel:
-        sel_deconv = deconv_e / deconv_e.max() > e_cut
-    else:
-        raise ValueError(f'cut_type {cut_type} is not a valid cut type.')
+    if   cut_type is CutType.abs: sel_deconv = deconv_e                  > e_cut
+    else                        : sel_deconv = deconv_e / deconv_e.max() > e_cut
 
     df['E']     = deconv_e[sel_deconv]
     df['event'] = hits.event.unique()[0]
@@ -497,9 +491,10 @@ def beersheba( files_in         : OneOrManyFiles
 
     for p in ['sample_width', 'bin_size', 'diffusion']:
         if len(deconv_params[p]) != deconv_params['n_dim']:
-            raise ValueError         (f"Parameter {p} dimensions do not match n_dim parameter")
+            raise ValueError(f"Parameter {p} dimensions do not match n_dim parameter")
+
     if deconv_params['n_dim'] > 2:
-        raise     NotImplementedError(f"{deconv_params['n_dim']}-dimensional PSF not yet implemented")
+        raise NotImplementedError(f"{deconv_params['n_dim']}-dimensional PSF not yet implemented")
 
     cut_sensors           = fl.map(cut_over_Q   (deconv_params.pop("q_cut")    , ['E', 'Ec']),
                                    item = 'hits')

invisible_cities/cities/beersheba_test.py

@@ -20,28 +20,22 @@
 from .. types.symbols      import CutType
 
 
-def test_create_deconvolution_df(ICDATADIR):
+@mark.parametrize("cut_type", CutType)
+def test_create_deconvolution_df(ICDATADIR, cut_type):
     true_in  = os.path.join(ICDATADIR, "exact_Kr_deconvolution_with_MC.h5")
     true_dst = dio.load_dst(true_in, 'DECO', 'Events')
     ecut     = 1e-2
     new_dst  = pd.concat([create_deconvolution_df(t, t.E.values, (t.X.values, t.Y.values, t.Z.values),
-                                                  CutType.abs, ecut, 3) for _, t in true_dst.groupby('event')])
-    true_dst = true_dst.loc[true_dst.E > ecut, :].reset_index(drop=True)
-    # compare only existing columns
-    true_dst = true_dst.loc[:, new_dst.columns.values.tolist()]
-    assert_dataframes_close(new_dst .reset_index(drop=True), true_dst.reset_index(drop=True))
+                                                  cut_type, ecut, 3) for _, t in true_dst.groupby('event')])
+    for event, df in true_dst.groupby("event"):
+        sel = (df.E > ecut if cut_type is CutType.abs else
+               df.E > ecut * df.E.max())
+        df = df.loc[sel].reset_index(drop=True)
+        # compare only existing columns
+        df = df.loc[:, new_dst.columns.values.tolist()]
 
-
-@mark.parametrize("cut_type", CutType.__members__)
-def test_create_deconvolution_df_cuttype(ICDATADIR, cut_type):
-    true_in  = os.path.join(ICDATADIR, "exact_Kr_deconvolution_with_MC.h5")
-    true_dst = dio.load_dst(true_in, 'DECO', 'Events')
-    ecut     = 1e-2
-
-    with raises(ValueError):
-        create_deconvolution_df(true_dst, true_dst.E.values,
-                                (true_dst.X.values, true_dst.Y.values, true_dst.Z.values),
-                                cut_type, ecut, 3)
+        new_event = new_dst.loc[new_dst.event==event]
+        assert_dataframes_close(new_event.reset_index(drop=True), df.reset_index(drop=True))
 
 
 def test_distribute_energy(ICDATADIR):
@@ -56,6 +50,18 @@ def test_distribute_energy(ICDATADIR):
     assert np.isclose (true_dst1.E.sum(), true_dst2.E.sum())
 
 
+def test_beersheba_ndim(beersheba_config, config_tmpdir):
+    path_out = os.path.join(config_tmpdir, "beersheba_ndim.h5")
+    beersheba_config.update(dict(file_out = path_out))
+    beersheba_config["deconv_params"].update(dict( n_dim        = 3
+                                                 , sample_width = [1]*3
+                                                 , bin_size     = [1]*3
+                                                 , diffusion    = [1]*3 ))
+
+    with raises(NotImplementedError):
+        beersheba(**beersheba_config)
+
+
 @ignore_warning.no_config_group
 @ignore_warning.str_length
 @ignore_warning.not_kdst
@@ -107,6 +113,23 @@ def test_beersheba_exact_result( deco
                 expected = getattr(true_output_file.root, table)
                 assert_tables_equality(got, expected, rtol=1e-6)
 
+@mark.parametrize( "updates"
+                 , ( dict(sample_width = [1]), dict(sample_width = [1,1,1])
+                   , dict(bin_size     = [1]), dict(bin_size     = [1,1,1])
+                   , dict(diffusion    = [1]), dict(diffusion    = [1,1,1])
+                    ))
+def test_beersheba_checks_dimensions(config_tmpdir, beersheba_config, updates):
+    # check that if these quantities do not have the correct number of
+    # dimensions (2), the city raises an error
+    par = next(iter(updates))
+    n   = len(updates[par])
+    path_out = os.path.join(config_tmpdir, f"beersheba_only_check_dims_{par}_{n}.h5")
+    beersheba_config.update(dict(file_out = path_out))
+    beersheba_config['deconv_params'].update(updates)
+
+    with raises(ValueError, match="Parameter .* dimensions do not match n_dim parameter"):
+        beersheba(**beersheba_config)
+
 
 @ignore_warning.no_config_group
 @ignore_warning.str_length