Merge pull request #154 from ACCLAB/refactoring_phase2

Refactoring phase2

Author: maiyishan

.github/workflows/test-pytest.yaml

@@ -15,4 +15,4 @@ jobs:
         run: |
           python -m pip install --upgrade pip
           pip install -e '.[dev]'
-          pytest nbs/tests/ --mpl --mpl-baseline-path=nbs/tests/baseline_images
+          pytest nbs/tests/ --mpl --mpl-baseline-path=nbs/tests/mpl_image_tests/baseline_images

dabest/_api.py

@@ -128,13 +128,15 @@ def prop_dataset(
         elif not set(group_names) == set(group.keys()):
             # Check if the group_names provided is the same as the keys of the dict
             raise ValueError("group_names must be the same as the keys of the dict.")
+        
         # Check if the values in the dict are numeric
         if not all(
             [isinstance(group[name], (list, tuple, np.ndarray)) for name in group_names]
         ):
             raise ValueError(
                 "group must be a dict of lists, tuples, or numpy ndarrays of numeric types."
             )
+        
         # Check if the values in the dict only have two elements under each parent key
         if not all([len(group[name]) == 2 for name in group_names]):
             raise ValueError("Each parent key should have only two elements.")
@@ -143,6 +145,7 @@ def prop_dataset(
     else:
         if group_names is None:
             raise ValueError("group_names must be provided if group is not a dict.")
+        
         # Check if the length of group is two times of the length of group_names
         if not len(group) == 2 * len(group_names):
             raise ValueError(

dabest/_bootstrap_tools.py

@@ -88,10 +88,10 @@ def __init__(
             # check x2 is not None:
             if x2 is None:
                 raise ValueError("Please specify x2.")
-            else:
-                x2 = pd.Series(x2).dropna()
-                if len(x1) != len(x2):
-                    raise ValueError("x1 and x2 are not the same length.")
+            
+            x2 = pd.Series(x2).dropna()
+            if len(x1) != len(x2):
+                raise ValueError("x1 and x2 are not the same length.")
 
         if (x2 is None) or (paired is not None):
             if x2 is None:
@@ -102,7 +102,6 @@ def __init__(
                 ttest_2_paired = "NIL"
                 wilcoxonresult = "NIL"
 
-            # elif paired is not None:
             else:  # only two options to enter here
                 diff = True
                 tx = x2 - x1
@@ -234,18 +233,18 @@ def jackknife_indexes(data):
     return (np.delete(base, i) for i in base)
 
 
-def bca(data, alphas, statarray, stat_function, ostat, reps):
+def bca(data, alphas, stat_array, stat_function, ostat, reps):
     """
     Subroutine called to calculate the BCa statistics.
     Borrowed heavily from scikits.bootstrap code.
     """
 
     # The bias correction value.
-    z0 = norm.ppf((1.0 * np.sum(statarray < ostat, axis=0)) / reps)
+    z0 = norm.ppf((1.0 * np.sum(stat_array < ostat, axis=0)) / reps)
 
     # Statistics of the jackknife distribution
-    jackindexes = jackknife_indexes(data[0])
-    jstat = [stat_function(*(x[indexes] for x in data)) for indexes in jackindexes]
+    jack_indexes = jackknife_indexes(data[0])
+    jstat = [stat_function(*(x[indexes] for x in data)) for indexes in jack_indexes]
     jmean = np.mean(jstat, axis=0)
 
     # Acceleration value

dabest/_dabest_object.py

@@ -54,120 +54,16 @@ def __init__(
         self.__proportional = proportional
         self.__mini_meta = mini_meta
 
-        # Check if it is a valid mini_meta case
-        if self.__mini_meta:
-            # Only mini_meta calculation but not proportional and delta-delta function
-            if self.__proportional:
-                err0 = "`proportional` and `mini_meta` cannot be True at the same time."
-                raise ValueError(err0)
-            if self.__delta2:
-                err0 = "`delta` and `mini_meta` cannot be True at the same time."
-                raise ValueError(err0)
-
-            # Check if the columns stated are valid
-            # TODO instead of traversing twice idx you can traverse only once
-            # and break the loop if the condition is not satisfied?
-            # TODO What if the type is not str and not tuple,list? missing raise Error
-            if all([isinstance(i, str) for i in idx]):
-                if len(pd.unique([t for t in idx]).tolist()) != 2:
-                    err0 = "`mini_meta` is True, but `idx` ({})".format(idx)
-                    err1 = "does not contain exactly 2 columns."
-                    raise ValueError(err0 + err1)
-
-            if all([isinstance(i, (tuple, list)) for i in idx]):
-                all_idx_lengths = [len(t) for t in idx]
-                if (array(all_idx_lengths) != 2).any():
-                    err1 = "`mini_meta` is True, but some idx "
-                    err2 = "in {} does not consist only of two groups.".format(idx)
-                    raise ValueError(err1 + err2)
-
-        # TODO can you have True mini_meta and delta2 at the same time?
-        # Check if this is a 2x2 ANOVA case and x & y are valid columns
-        # Create experiment_label and x1_level
-        if self.__delta2:
-            # TODO Wrap the errors in a separate function called check_errors()
-            if self.__proportional:
-                err0 = "`proportional` and `delta` cannot be True at the same time."
-                raise ValueError(err0)
-
-            # idx should not be specified
-            if idx:
-                err0 = "`idx` should not be specified when `delta2` is True.".format(
-                    len(x)
-                )
-                raise ValueError(err0)
-
-            # Check if x is valid
-            # TODO if x is None is fine??
-            if len(x) != 2:
-                err0 = "`delta2` is True but the number of variables indicated by `x` is {}.".format(
-                    len(x)
-                )
-                raise ValueError(err0)
-
-            for i in x:
-                if i not in self.__output_data.columns:
-                    err = "{0} is not a column in `data`. Please check.".format(i)
-                    raise IndexError(err)
-
-            # Check if y is valid
-            if not y:
-                err0 = "`delta2` is True but `y` is not indicated."
-                raise ValueError(err0)
-
-            if y not in self.__output_data.columns:
-                err = "{0} is not a column in `data`. Please check.".format(y)
-                raise IndexError(err)
-
-            # Check if experiment is valid
-            if experiment not in self.__output_data.columns:
-                err = "{0} is not a column in `data`. Please check.".format(experiment)
-                raise IndexError(err)
-
-            # Check if experiment_label is valid and create experiment when needed
-            if experiment_label:
-                if len(experiment_label) != 2:
-                    err0 = "`experiment_label` does not have a length of 2."
-                    raise ValueError(err0)
-
-                for i in experiment_label:
-                    if i not in self.__output_data[experiment].unique():
-                        err = "{0} is not an element in the column `{1}` of `data`. Please check.".format(
-                            i, experiment
-                        )
-                        raise IndexError(err)
-            else:
-                experiment_label = self.__output_data[experiment].unique()
-
-            # Check if x1_level is valid
-            if x1_level:
-                if len(x1_level) != 2:
-                    err0 = "`x1_level` does not have a length of 2."
-                    raise ValueError(err0)
-
-                for i in x1_level:
-                    if i not in self.__output_data[x[0]].unique():
-                        err = "{0} is not an element in the column `{1}` of `data`. Please check.".format(
-                            i, experiment
-                        )
-                        raise IndexError(err)
-
-            else:
-                x1_level = self.__output_data[x[0]].unique()
-
-        # TODO what if experiment is None?
-        elif experiment:
-            experiment_label = self.__output_data[experiment].unique()
-            x1_level = self.__output_data[x[0]].unique()
-        self.__experiment_label = experiment_label
-        self.__x1_level = x1_level
+        # after this call the attributes self.__experiment_label and self.__x1_level are updated
+        self._check_errors(x, y, idx, experiment, experiment_label, x1_level)
+        
 
         # create new x & idx and record the second variable if this is a valid 2x2 ANOVA case
         if idx is None and x is not None and y is not None:
             # Add a length check for unique values in the first element in list x,
             # if the length is greater than 2, force delta2 to be False
             # Should be removed if delta2 for situations other than 2x2 is supported
-            if len(self.__output_data[x[0]].unique()) > 2 and x1_level is None:
+            if len(self.__output_data[x[0]].unique()) > 2 and self.__x1_level is None:
                 self.__delta2 = False
                 # stop the loop if delta2 is False
 
@@ -184,9 +80,9 @@ def __init__(
 
             # create idx and record the first and second x variable
             idx = []
-            for i in list(map(lambda x: str(x), experiment_label)):
+            for i in list(map(lambda x: str(x), self.__experiment_label)):
                 temp = []
-                for j in list(map(lambda x: str(x), x1_level)):
+                for j in list(map(lambda x: str(x), self.__x1_level)):
                     temp.append(j + " " + i)
                 idx.append(temp)
 
@@ -242,7 +138,7 @@ def __init__(
             err = "You have only specified `x`. Please also specify `y`."
             raise ValueError(err)
 
-        self.__plot_data = self.get_plot_data(x, y, all_plot_groups)
+        self.__plot_data = self._get_plot_data(x, y, all_plot_groups)
         self.__all_plot_groups = all_plot_groups
 
         # Check if `id_col` is valid
@@ -529,7 +425,123 @@ def _all_plot_groups(self):
         """
         return self.__all_plot_groups
 
-    def get_plot_data(self, x, y, all_plot_groups):
+    def _check_errors(self, x, y, idx, experiment, experiment_label, x1_level):
+        '''
+        Function to check some input parameters and combinations between them.
+        At the end of this function these two class attributes are updated
+                self.__experiment_label and self.__x1_level
+        '''
+        # Check if it is a valid mini_meta case
+        if self.__mini_meta:
+            # Only mini_meta calculation but not proportional and delta-delta function
+            if self.__proportional:
+                err0 = "`proportional` and `mini_meta` cannot be True at the same time."
+                raise ValueError(err0)
+            if self.__delta2:
+                err0 = "`delta2` and `mini_meta` cannot be True at the same time."
+                raise ValueError(err0)
+
+            # Check if the columns stated are valid
+            # TODO instead of traversing twice idx you can traverse only once
+            # and break the loop if the condition is not satisfied?
+            # TODO What if the type is not str and not tuple,list? missing raise Error
+            if all([isinstance(i, str) for i in idx]):
+                if len(pd.unique([t for t in idx]).tolist()) != 2:
+                    err0 = "`mini_meta` is True, but `idx` ({})".format(idx)
+                    err1 = "does not contain exactly 2 columns."
+                    raise ValueError(err0 + err1)
+
+            if all([isinstance(i, (tuple, list)) for i in idx]):
+                all_idx_lengths = [len(t) for t in idx]
+                if (array(all_idx_lengths) != 2).any():
+                    err1 = "`mini_meta` is True, but some idx "
+                    err2 = "in {} does not consist only of two groups.".format(idx)
+                    raise ValueError(err1 + err2)
+
+        # TODO can you have True mini_meta and delta2 at the same time?
+        # Check if this is a 2x2 ANOVA case and x & y are valid columns
+        # Create experiment_label and x1_level
+        if self.__delta2:
+            if x is None:
+                error_msg = "If `delta2` is True. `x` parameter cannot be None. String or list expected"
+                raise ValueError(error_msg)
+                
+            if self.__proportional:
+                err0 = "`proportional` and `delta2` cannot be True at the same time."
+                raise ValueError(err0)
+
+            # idx should not be specified
+            if idx:
+                err0 = "`idx` should not be specified when `delta2` is True.".format(
+                    len(x)
+                )
+                raise ValueError(err0)
+
+            # Check if x is valid
+            if len(x) != 2:
+                err0 = "`delta2` is True but the number of variables indicated by `x` is {}.".format(
+                    len(x)
+                )
+                raise ValueError(err0)
+
+            for i in x:
+                if i not in self.__output_data.columns:
+                    err = "{0} is not a column in `data`. Please check.".format(i)
+                    raise IndexError(err)
+
+            # Check if y is valid
+            if not y:
+                err0 = "`delta2` is True but `y` is not indicated."
+                raise ValueError(err0)
+
+            if y not in self.__output_data.columns:
+                err = "{0} is not a column in `data`. Please check.".format(y)
+                raise IndexError(err)
+
+            # Check if experiment is valid
+            if experiment not in self.__output_data.columns:
+                err = "{0} is not a column in `data`. Please check.".format(experiment)
+                raise IndexError(err)
+
+            # Check if experiment_label is valid and create experiment when needed
+            if experiment_label:
+                if len(experiment_label) != 2:
+                    err0 = "`experiment_label` does not have a length of 2."
+                    raise ValueError(err0)
+
+                for i in experiment_label:
+                    if i not in self.__output_data[experiment].unique():
+                        err = "{0} is not an element in the column `{1}` of `data`. Please check.".format(
+                            i, experiment
+                        )
+                        raise IndexError(err)
+            else:
+                experiment_label = self.__output_data[experiment].unique()
+
+            # Check if x1_level is valid
+            if x1_level:
+                if len(x1_level) != 2:
+                    err0 = "`x1_level` does not have a length of 2."
+                    raise ValueError(err0)
+
+                for i in x1_level:
+                    if i not in self.__output_data[x[0]].unique():
+                        err = "{0} is not an element in the column `{1}` of `data`. Please check.".format(
+                            i, experiment
+                        )
+                        raise IndexError(err)
+
+            else:
+                x1_level = self.__output_data[x[0]].unique()
+
+        # TODO what if experiment is None?
+        elif experiment:
+            experiment_label = self.__output_data[experiment].unique()
+            x1_level = self.__output_data[x[0]].unique()
+        self.__experiment_label = experiment_label
+        self.__x1_level = x1_level
+
+    def _get_plot_data(self, x, y, all_plot_groups):
         """
         Function to prepare some attributes for plotting
         """

dabest/_delta_objects.py

@@ -409,8 +409,7 @@ def __init__(self, effectsizedataframe, permutation_count,
         # weights
         self.__bootstraps_weighted_delta = ci2g.calculate_weighted_delta(
                                                           self.__group_var, 
-                                                          self.__bootstraps, 
-                                                          self.__resamples)
+                                                          self.__bootstraps)
 
         # Compute the weighted average mean difference based on the raw data
         self.__difference = es.weighted_delta(self.__effsizedf["difference"],

dabest/_effsize_objects.py

@@ -663,10 +663,12 @@ def permutations(self):
 
     @property
     def permutations_var(self):
+         # TODO Missing docstring
         return self.__PermutationTest_result.permutations_var
 
     @property
     def proportional_difference(self):
+         # TODO Missing docstring
         try:
             return self.__proportional_difference
         except AttributeError:
@@ -1139,7 +1141,7 @@ def plot(self, color_col=None,
 
         """
 
-        from .plotter import EffectSizeDataFramePlotter
+        from .plotter import effectsize_df_plotter
 
         if hasattr(self, "results") is False:
             self.__pre_calc()
@@ -1156,7 +1158,7 @@ def plot(self, color_col=None,
         all_kwargs = locals()
         del all_kwargs["self"]
 
-        out = EffectSizeDataFramePlotter(self, **all_kwargs)
+        out = effectsize_df_plotter(self, **all_kwargs)
 
         return out
 

dabest/_modidx.py

@@ -78,5 +78,4 @@
                                    'dabest.plot_tools.sankeydiag': ('API/plot_tools.html#sankeydiag', 'dabest/plot_tools.py'),
                                    'dabest.plot_tools.single_sankey': ('API/plot_tools.html#single_sankey', 'dabest/plot_tools.py'),
                                    'dabest.plot_tools.width_determine': ('API/plot_tools.html#width_determine', 'dabest/plot_tools.py')},
-            'dabest.plotter': { 'dabest.plotter.EffectSizeDataFramePlotter': ( 'API/plotter.html#effectsizedataframeplotter',
-                                                                               'dabest/plotter.py')}}}
+            'dabest.plotter': {'dabest.plotter.effectsize_df_plotter': ('API/plotter.html#effectsize_df_plotter', 'dabest/plotter.py')}}}