Repository: mindsdb/mindsdb
Pull request: Hotfix: Unit test file structure, pytest back to travis

Problem statement:
Figure out the issues related to data analysis v2
Since the 1.21.0 release @setohe0909 has reported some issues, we should sync with him to figure out what they are and how to solve them.

Issues running unit tests in travis
At the moment travis seems to silently fail if I add `pytests` to the `.travis.yml`, but I see no relevant logs.

Running it locally I get the error:

```
ERROR: usage: pytest [options] [file_or_dir] [file_or_dir] [...]
pytest: error: unrecognized arguments: --randomly-seed=42
  inifile: /home/george/mindsdb/pytest.ini
  rootdir: /home/george/mindsdb
```

Maybe it's an issue to do with the specific version being used ?

If so we should try to support the latest (>= 5.4.x) and explicitly ask for that to be installed when running the test on travis.

Unit test placement
Currently the unit tests are placed in a "flat" directory structure inside `tests/unit_tests`.

Ideally they would follow the current pattern and be at the bottom of the file they are testing under a `if __name__ == '__main__'` check.

If this is not possible for some reason then we should have a 1-to-1 mapping of the mindsdb directory structure under `tests/unit_tests`, though this would be a less-then-ideal solution.

Hints from issue discussion:
I have am 100% against the tests being in the same thing they are testing.

It goes against all conventions, completely. Nobody does that.
Tests are a lot of code. Tests can easily grow way larger than the thing they are testing. It's a lot of useless clutter for developers. Also if you are distributing the code somehow, you don't want to bundle tens of thousand of test code lines together with the main code. Also moving the tests will screw up pytest test discovery. Also it will screw up pytest `conftest.py` convention, that is pytest won't find where it should take the test fixtures from.

See this:
https://docs.pytest.org/en/latest/goodpractices.html

In my opinion, a flat directory structure is fine until it gets too cumbersome. When it gets cumbersome we can get some dirs to organize them, for example put phases tests inside `test/phases`. This is easy to do at any point since test location doesn't matter at all except for readability, so there is no need to do it now.

Commit message: New unit test structure, readme additions, pytest back to travis, add test requirements to setup.py tests_require

Patch to review:
diff --git a/.travis.yml b/.travis.yml
index 15170a8dca5..f95f309f907 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -55,8 +55,8 @@ install:
   - if [ "$TRAVIS_OS_NAME" = "windows" ]; then travis_wait 15 pip install --no-cache-dir -e .; fi
   - if [ "$TRAVIS_OS_NAME" != "windows" ]; then travis_wait 15 pip3 install --no-cache-dir -e .; fi
 # run tests
-#  Add when fixed: - pytest -s
 script:
+  - pytest -s
   - cd tests/ci_tests
   - if [ "$TRAVIS_OS_NAME" = "windows" ]; then travis_wait 15 python fast_test.py; fi
   - if [ "$TRAVIS_OS_NAME" != "windows" ]; then travis_wait 15 python3 full_test.py; fi
diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
index 88b71d18d8b..c7f871adf83 100644
--- a/CONTRIBUTING.md
+++ b/CONTRIBUTING.md
@@ -20,7 +20,8 @@ In general, we follow the "fork-and-pull" Git workflow.
 4. Push your local branch to your fork
 5. Submit a Pull request so that we can review your changes
 6. Write a commit message
-7. Make sure that the CI tests are GREEN
+7. Make sure that the unit tests are GREEN
+8. Make sure that the CI tests are GREEN
 
 >NOTE: Be sure to merge the latest from "upstream" before making a pull request!
 
diff --git a/README.md b/README.md
index 142ff85d5b0..2493075207f 100755
--- a/README.md
+++ b/README.md
@@ -140,7 +140,13 @@ In order to make changes to mindsdb, the ideal approach is to fork the repositor
 
 For example: `export PYTHONPATH=$PYTHONPATH:/home/my_username/mindsdb`.
 
-To test if your changes are working you can try running the CI tests locally: `cd tests/ci_tests && python3 full_test.py`
+Too test your changes you can run unit tests (fast) and CI tests (slightly longer) locally.
+
+To run the unit tests:
+* Install pytest: `pip install -r requirements_test.txt`
+* Run: `pytest`
+
+To run the CI tests: `cd tests/ci_tests && python3 full_test.py`
 
 Once you have specific changes you want to merge into master, feel free to make a PR.
 
diff --git a/mindsdb/libs/helpers/general_helpers.py b/mindsdb/libs/helpers/general_helpers.py
index cb00099a89a..bafa433c31d 100755
--- a/mindsdb/libs/helpers/general_helpers.py
+++ b/mindsdb/libs/helpers/general_helpers.py
@@ -117,6 +117,7 @@ def closest(arr, value):
     return len(arr)-1
 
 
+# @TODO not pass huge dicts of stats to this function, just pass the data type
 def get_value_bucket(value, buckets, col_stats, hmd=None):
     """
     :return: The bucket in the `histogram` in which our `value` falls
@@ -132,8 +133,8 @@ def get_value_bucket(value, buckets, col_stats, hmd=None):
 
     elif col_stats['data_subtype'] in (DATA_SUBTYPES.BINARY, DATA_SUBTYPES.INT, DATA_SUBTYPES.FLOAT):
         bucket = closest(buckets, value)
-    elif col_stats['data_subtype'] in (DATA_SUBTYPES.IMAGE) and hmd is not None:
-        bucket = hmd['bucketing_algorithms'][col_name].predict(np.array(imagehash.phash(Image.open(value)).reshape(1, -1)))[0]
+    # elif col_stats['data_subtype'] in (DATA_SUBTYPES.IMAGE) and hmd is not None:
+    #     bucket = hmd['bucketing_algorithms'][col_name].predict(np.array(imagehash.phash(Image.open(value)).reshape(1, -1)))[0]
     else:
         bucket = len(buckets) # for null values
 
diff --git a/mindsdb/libs/phases/data_analyzer/data_analyzer.py b/mindsdb/libs/phases/data_analyzer/data_analyzer.py
index bf55e77e73f..cea84a387ce 100755
--- a/mindsdb/libs/phases/data_analyzer/data_analyzer.py
+++ b/mindsdb/libs/phases/data_analyzer/data_analyzer.py
@@ -165,6 +165,7 @@ def get_uniq_values_report(data):
 
     return ud
 
+
 def compute_entropy_biased_buckets(hist_y, hist_x):
     S, biased_buckets = None, None
     nr_values = sum(hist_y)
@@ -175,13 +176,51 @@ def compute_entropy_biased_buckets(hist_y, hist_x):
     return S, biased_buckets
 
 
+def compute_outlier_buckets(outlier_values,
+                            hist_x,
+                            hist_y,
+                            percentage_buckets,
+                            col_stats):
+    outlier_buckets = []
+    # map each bucket to list of outliers in it
+    bucket_outliers = defaultdict(list)
+    for value in outlier_values:
+        vb_index = get_value_bucket(value,
+                                    percentage_buckets,
+                                    col_stats)
+        vb = percentage_buckets[vb_index]
+        bucket_outliers[vb].append(value)
+
+    # Filter out buckets without outliers,
+    # then sort by number of outliers in ascending order
+    buckets_with_outliers = sorted(filter(
+        lambda kv: len(kv[1]) > 0, bucket_outliers.items()
+    ), key=lambda kv: len(kv[1]))
+
+    for i, (bucket, outlier_values) in enumerate(buckets_with_outliers):
+        bucket_index = hist_x.index(bucket)
+
+        bucket_values_num = hist_y[bucket_index]
+        bucket_outliers_num = len(outlier_values)
+
+        # Is the bucket in the 95th percentile by number of outliers?
+        percentile_outlier = ((i + 1) / len(buckets_with_outliers)) >= 0.95
+
+        # Are half of values in the bucket outliers?
+        predominantly_outlier = (bucket_outliers_num / bucket_values_num) > 0.5
+
+        if predominantly_outlier or percentile_outlier:
+            outlier_buckets.append(bucket)
+    return outlier_buckets
+
+
 class DataAnalyzer(BaseModule):
     """
     The data analyzer phase is responsible for generating the insights we need about the data in order to vectorize it.
     Additionally, also provides the user with some extra meaningful information about his data.
     """
 
-    # TODO get rid of scores entirely
+    # @TODO get rid of scores and stats entirely
     def compute_scores(self, col_name, sample_df, full_df, stats):
         for score_func in [compute_duplicates_score,
                            compute_empty_cells_score,
@@ -256,53 +295,24 @@ def run(self, input_data):
                     stats_v2[col_name]['bias']['biased_buckets'] = biased_buckets
                 if S < 0.8:
                     if data_type in (DATA_TYPES.CATEGORICAL):
-                        stats_v2[col_name]['bias']['warning'] =  """You may to check if some categories occur too often to too little in this columns. This doesn't necessarily mean there's an issue with your data, it just indicates a higher than usual probability there might be some issue."""
+                        warning_str =  "You may to check if some categories occur too often to too little in this columns."
                     else:
-                        stats_v2[col_name]['bias']['warning'] = """You may want to check if you see something suspicious on the right-hand-side graph. This doesn't necessarily mean there's an issue with your data, it just indicates a higher than usual probability there might be some issue"""
-
+                        warning_str = "You may want to check if you see something suspicious on the right-hand-side graph."
+                    stats_v2[col_name]['bias']['warning'] = warning_str + " This doesn't necessarily mean there's an issue with your data, it just indicates a higher than usual probability there might be some issue."
 
             self.compute_scores(col_name, sample_df, col_data_dict, stats)
 
             if 'lof_outliers' in stats[col_name]:
-                if data_subtype in (DATA_SUBTYPES.INT):
-                    stats[col_name]['lof_outliers'] = [int(x) for x in stats[col_name]['lof_outliers']]
-
                 stats_v2[col_name]['outliers'] = {
-                    'outlier_values': stats[col_name]['lof_outliers']
-                    ,'outlier_score': stats[col_name]['lof_based_outlier_score']
-                    ,'description': 'TBD'
+                    'outlier_values': stats[col_name]['lof_outliers'],
+                    'outlier_score': stats[col_name]['lof_based_outlier_score'],
+                    'outlier_buckets': compute_outlier_buckets(outlier_values=stats[col_name]['lof_outliers'],
+                                                               hist_x=histogram['x'],
+                                                               hist_y=histogram['y'],
+                                                               col_stats=stats[col_name]),
+                    'description': 'TBD'
                 }
 
-                # map each bucket to list of outliers in it
-                bucket_outliers = defaultdict(list)
-                for value in stats_v2[col_name]['outliers']['outlier_values']:
-                    vb_index = get_value_bucket(value, stats_v2[col_name]['percentage_buckets'], stats[col_name])
-                    vb = stats_v2[col_name]['percentage_buckets'][vb_index]
-                    bucket_outliers[vb].append(value)
-
-                # Filter out buckets without outliers,
-                # then sort by number of outliers in ascending order
-                buckets_with_outliers = sorted(filter(
-                    lambda kv: len(kv[1]) > 0, bucket_outliers.items()
-                ), key=lambda kv: len(kv[1]))
-
-                stats_v2[col_name]['outliers']['outlier_buckets'] = []
-
-                for i, (bucket, outlier_values) in enumerate(buckets_with_outliers):
-                    bucket_index = stats_v2[col_name]['histogram']['x'].index(bucket)
-
-                    bucket_values_num = stats_v2[col_name]['histogram']['y'][bucket_index]
-                    bucket_outliers_num = len(outlier_values)
-
-                    # Is the bucket in the 95th percentile by number of outliers?
-                    percentile_outlier = ((i + 1) / len(buckets_with_outliers)) >= 0.95
-
-                    # Are half of values in the bucket outliers?
-                    predominantly_outlier = (bucket_outliers_num / bucket_values_num) > 0.5
-
-                    if predominantly_outlier or percentile_outlier:
-                        stats_v2[col_name]['outliers']['outlier_buckets'].append(bucket)
-
             stats_v2[col_name]['nr_warnings'] = 0
             for x in stats_v2[col_name].values():
                 if isinstance(x, dict) and 'warning' in x:
diff --git a/setup.py b/setup.py
index 0ce913a2d9d..5f96cba66f2 100755
--- a/setup.py
+++ b/setup.py
@@ -23,6 +23,9 @@ def remove_requirements(requirements, name, replace=None):
 with open('requirements.txt', 'r') as req_file:
     requirements = [req.strip() for req in req_file.read().splitlines()]
 
+with open('requirements_tests.txt', 'r') as req_file:
+    test_requirements = [req.strip() for req in req_file.read().splitlines()]
+
 extra_data_sources_requirements = []
 with open('optional_requirements_extra_data_sources.txt', 'r') as fp:
     for line in fp:
@@ -77,6 +80,7 @@ def remove_requirements(requirements, name, replace=None):
         ,'ludwig_model': ludwig_model_requirements
         ,'beta': beta_requirements
     },
+    tests_require = test_requirements,
     dependency_links=dependency_links,
     classifiers=(
         "Programming Language :: Python :: 3",
diff --git a/test.py b/test.py
index 7b066e6b62c..9419eb06b0a 100644
--- a/test.py
+++ b/test.py
@@ -5,22 +5,31 @@
 import time
 import lightwood
 
-
 lightwood.config.config.CONFIG.HELPER_MIXERS = False
 mdb = Predictor(name='test_predictor')
-#'rental_price',
-#mdb.learn(to_predict=['neighborhood','rental_price'],from_data="https://mindsdb-example-data.s3.eu-west-2.amazonaws.com/home_rentals.csv",use_gpu=False,stop_training_in_x_seconds=1000, backend='lightwood', unstable_parameters_dict={'use_selfaware_model':True})
-
-p = mdb.predict(when={'number_of_rooms': 3, 'number_of_bathrooms': 2, 'sqft':2411, 'initial_price':3000}, run_confidence_variation_analysis=True, use_gpu=True)
+# 'rental_price',
+mdb.learn(to_predict=['neighborhood', 'rental_price'],
+          from_data="https://mindsdb-example-data.s3.eu-west-2.amazonaws.com/home_rentals.csv",
+          use_gpu=False, stop_training_in_x_seconds=1000, backend='lightwood',
+          unstable_parameters_dict={'use_selfaware_model': True})
+
+p = mdb.predict(
+    when={'number_of_rooms': 3, 'number_of_bathrooms': 2, 'sqft': 2411,
+          'initial_price': 3000}, run_confidence_variation_analysis=True,
+    use_gpu=False)
 e = p[0].explanation
 
-p_arr = mdb.predict(when_data='https://mindsdb-example-data.s3.eu-west-2.amazonaws.com/home_rentals.csv', use_gpu=True)
+p_arr = mdb.predict(
+    when_data='https://mindsdb-example-data.s3.eu-west-2.amazonaws.com/home_rentals.csv',
+    use_gpu=False)
 print(p_arr[0].explanation)
 
 for p in p_arr:
     e = p.explanation
 
-p = mdb.predict(when={'number_of_rooms': 3, 'number_of_bathrooms': 2, 'neighborhood': 'south_side', 'sqft':2411}, run_confidence_variation_analysis=True, use_gpu=True)
+p = mdb.predict(when={'number_of_rooms': 3, 'number_of_bathrooms': 2,
+                      'neighborhood': 'south_side', 'sqft': 2411},
+                run_confidence_variation_analysis=True, use_gpu=False)
 
 for p in p_arr:
     exp_s = p.epitomize()
diff --git a/tests/unit_tests/README.md b/tests/README.md
similarity index 86%
rename from tests/unit_tests/README.md
rename to tests/README.md
index 80f93923db0..5e44a11276a 100644
--- a/tests/unit_tests/README.md
+++ b/tests/README.md
@@ -1,15 +1,19 @@
+# CI tests
+Running CI tests locally:
+
+`cd tests/ci_tests && python3 full_test.py`
+
+# Unit tests
 Unit tests use the [pytest](https://docs.pytest.org/en/latest/index.html) testing framework. [pytest-randomly](https://pypi.org/project/pytest-randomly/) is used to fix random seed for reproducibility.
 
----
-# Test guidelines
+## Test guidelines
 * Each unit test should ideally test just one thing.
 * Unit tests should be easy to read, even if it requires some code duplication.
 * Unit tests should be as simple as possible, so they don't become a burden to support.
 
 [More good unit testing practices](https://pylonsproject.org/community-unit-testing-guidelines.html).
 
----
-# Useful commands
+## Useful commands
 
 Run tests until first failure:
 
diff --git a/tests/unit_tests/__init__.py b/tests/__init__.py
similarity index 100%
rename from tests/unit_tests/__init__.py
rename to tests/__init__.py
diff --git a/tests/unit_tests/test_data_analyzer.py b/tests/unit_tests/libs/phases/data_analyzer/test_data_analyzer.py
similarity index 99%
rename from tests/unit_tests/test_data_analyzer.py
rename to tests/unit_tests/libs/phases/data_analyzer/test_data_analyzer.py
index 303ee36b7fb..10ff14b162e 100644
--- a/tests/unit_tests/test_data_analyzer.py
+++ b/tests/unit_tests/libs/phases/data_analyzer/test_data_analyzer.py
@@ -9,7 +9,7 @@
 from mindsdb.libs.constants.mindsdb import DATA_TYPES, DATA_SUBTYPES
 from mindsdb.libs.data_types.transaction_data import TransactionData
 from mindsdb.libs.phases.data_analyzer.data_analyzer import DataAnalyzer
-from .utils import test_column_types
+from tests.unit_tests.utils import test_column_types
 
 
 class TestDataAnalyzer:
diff --git a/tests/unit_tests/test_type_deductor.py b/tests/unit_tests/libs/phases/type_deductor/test_type_deductor.py
similarity index 99%
rename from tests/unit_tests/test_type_deductor.py
rename to tests/unit_tests/libs/phases/type_deductor/test_type_deductor.py
index 73f248bc0ca..0b8f35bcf45 100644
--- a/tests/unit_tests/test_type_deductor.py
+++ b/tests/unit_tests/libs/phases/type_deductor/test_type_deductor.py
@@ -8,7 +8,7 @@
 from mindsdb.libs.constants.mindsdb import DATA_TYPES, DATA_SUBTYPES
 from mindsdb.libs.data_types.transaction_data import TransactionData
 from mindsdb.libs.phases.type_deductor.type_deductor import TypeDeductor
-from .utils import test_column_types
+from tests.unit_tests.utils import test_column_types
 
 
 class TestTypeDeductor:


Code review comments:
diff_hunk: @@ -132,8 +133,8 @@ def get_value_bucket(value, buckets, col_stats, hmd=None):
 
     elif col_stats['data_subtype'] in (DATA_SUBTYPES.BINARY, DATA_SUBTYPES.INT, DATA_SUBTYPES.FLOAT):
         bucket = closest(buckets, value)
-    elif col_stats['data_subtype'] in (DATA_SUBTYPES.IMAGE) and hmd is not None:
-        bucket = hmd['bucketing_algorithms'][col_name].predict(np.array(imagehash.phash(Image.open(value)).reshape(1, -1)))[0]
+    # elif col_stats['data_subtype'] in (DATA_SUBTYPES.IMAGE) and hmd is not None:
path: mindsdb/libs/helpers/general_helpers.py
text: @user1:
Why is this commented out ?

@author:
Its broken. `col_name` is not defined in this function

@author:
On another thought, I will uncomment it and make an issue about it

@author:
https://github.com/mindsdb/mindsdb/issues/485