Repository: biocore/microsetta-public-api

Issue:
The Taxonomy model is initialized on each GET and it's very expensive
Each GET seems to initialize the `Taxonomy` model, and takes about 2 minutes on my laptop. The interaction with this object is read-only -- is there a reason it needs to be re-initialized on each call rather than being done once at server start up?

Discussion:

Patch:
diff --git a/README.md b/README.md
index e690226..c3dc94f 100644
--- a/README.md
+++ b/README.md
@@ -50,6 +50,9 @@ feature data. Tables can either be QZA `FeatureTable`'s or biom tables. If a `bi
 the `"table-format": "biom"` option must be specified. Taxonomy data is accepted with the
 `"feature-data-taxonomy"` keyword, which should correspond to the filepath to a QIIME2 `FeatureData[Taxonomy]`.
 
+For large datasets, creating a Taxonomy model can be slow. You can cache the model on server startup by adding the
+`"cache-taxonomy": true` keyword to your specific table. This should speed up the time for API calls.
+
 ### Metadata
 The server can be configured to contain metadata, by adding a  `"metadata"` keyword to the config,
 that gives a path to a QIIME2 formatted metadata file.
@@ -66,7 +69,8 @@ that gives a path to a QIIME2 formatted metadata file.
     "taxonomy": {
       "table": "/path/to/table.biom",
       "table-format": "biom",
-      "feature-data-taxonomy": "/path/to/a/taxonomy-data.qza"
+      "feature-data-taxonomy": "/path/to/a/taxonomy-data.qza",
+      "cache-model": true
     }
   },
   "metadata": "/path/to/some/metadata.txt"
diff --git a/microsetta_public_api/api/metadata.py b/microsetta_public_api/api/metadata.py
index 26e8d6e..9b0d6ea 100644
--- a/microsetta_public_api/api/metadata.py
+++ b/microsetta_public_api/api/metadata.py
@@ -32,7 +32,7 @@ def filter_sample_ids(taxonomy=None, alpha_metric=None, **kwargs):
     )
 
     if error_response:
-        return error_response, error_code
+        return jsonify(*error_response), error_code
 
     return jsonify(sample_ids=matching_ids), 200
 
diff --git a/microsetta_public_api/api/taxonomy.py b/microsetta_public_api/api/taxonomy.py
index 95a33ea..777319c 100644
--- a/microsetta_public_api/api/taxonomy.py
+++ b/microsetta_public_api/api/taxonomy.py
@@ -1,6 +1,5 @@
 from microsetta_public_api.repo._taxonomy_repo import TaxonomyRepo
 from microsetta_public_api.utils import jsonify
-from microsetta_public_api.models._taxonomy import Taxonomy
 from microsetta_public_api.utils._utils import (validate_resource,
                                                 check_missing_ids,
                                                 )
@@ -33,11 +32,8 @@ def _summarize_group(sample_ids, table_name):
     if missing_ids_msg:
         return missing_ids_msg
 
-    table = taxonomy_repo.table(table_name)
-    features = taxonomy_repo.feature_data_taxonomy(table_name)
-    variances = taxonomy_repo.variances(table_name)
+    taxonomy_ = taxonomy_repo.model(table_name)
 
-    taxonomy_ = Taxonomy(table, features, variances)
     taxonomy_data = taxonomy_.get_group(sample_ids, '').to_dict()
     del taxonomy_data['name']
     del taxonomy_data['feature_ranks']
diff --git a/microsetta_public_api/repo/_taxonomy_repo.py b/microsetta_public_api/repo/_taxonomy_repo.py
index 250d2ea..49b015a 100644
--- a/microsetta_public_api/repo/_taxonomy_repo.py
+++ b/microsetta_public_api/repo/_taxonomy_repo.py
@@ -1,4 +1,5 @@
 from microsetta_public_api.resources import resources
+from microsetta_public_api.models._taxonomy import Taxonomy as TaxonomyModel
 
 
 class TaxonomyRepo:
@@ -59,6 +60,15 @@ class TaxonomyRepo:
     def variances(self, table_name):
         return self._get_resource(table_name, component='variances')
 
+    def model(self, table_name):
+        model = self._get_resource(table_name, component='model')
+        if model is None:
+            table = self.table(table_name)
+            features = self.feature_data_taxonomy(table_name)
+            variances = self.variances(table_name)
+            model = TaxonomyModel(table, features, variances)
+        return model
+
     def exists(self, sample_ids, table_name):
         """Checks if sample_ids exist for the given table.
 
diff --git a/microsetta_public_api/resources.py b/microsetta_public_api/resources.py
index 8365c35..23b78fd 100644
--- a/microsetta_public_api/resources.py
+++ b/microsetta_public_api/resources.py
@@ -10,6 +10,8 @@ from q2_types.sample_data import AlphaDiversity, SampleData
 from q2_types.feature_table import FeatureTable, Frequency
 from q2_types.feature_data import FeatureData, Taxonomy
 
+from microsetta_public_api.models._taxonomy import Taxonomy as TaxonomyModel
+
 
 def _dict_of_paths_to_alpha_data(dict_of_qza_paths, resource_name):
     _validate_dict_of_paths(dict_of_qza_paths,
@@ -42,7 +44,8 @@ def _transform_single_table(dict_, resource_name):
 
     _validate_dict_of_paths(dict_, resource_name, allow_none=True,
                             required_fields=['table'],
-                            non_ext_entries=['q2-type', 'table-format'],
+                            non_ext_entries=['q2-type', 'table-format',
+                                             'cache-taxonomy'],
                             allow_extras=True,
                             extensions=['.' + table_type]
                             )
@@ -85,6 +88,14 @@ def _transform_single_table(dict_, resource_name):
         elif key in biom_kws:
             new_resource[key] = biom.load_table(value)
 
+    cache_taxonomy = new_resource.get('cache-taxonomy', True)
+    if 'feature-data-taxonomy' in new_resource and cache_taxonomy:
+        table = new_resource['table']
+        taxonomy = new_resource['feature-data-taxonomy']
+        variances = new_resource.get('variances', None)
+        model = TaxonomyModel(table, taxonomy, variances)
+        new_resource['model'] = model
+
     return new_resource
 
 


Test patch:
diff --git a/microsetta_public_api/api/tests/implementation_tests/test_taxonomy.py b/microsetta_public_api/api/tests/implementation_tests/test_taxonomy.py
index b1c145c..12ed90f 100644
--- a/microsetta_public_api/api/tests/implementation_tests/test_taxonomy.py
+++ b/microsetta_public_api/api/tests/implementation_tests/test_taxonomy.py
@@ -9,6 +9,7 @@ from microsetta_public_api.utils.testing import MockedJsonifyTestCase
 from microsetta_public_api.api.taxonomy import (
     resources, summarize_group, _summarize_group, single_sample,
 )
+from microsetta_public_api.models._taxonomy import Taxonomy as TaxonomyModel
 
 
 class TaxonomyImplementationTests(MockedJsonifyTestCase):
@@ -276,6 +277,37 @@ class TaxonomyImplementationTests(MockedJsonifyTestCase):
                         obs['feature_variances']
                         )
 
+    def test_taxonomy_from_list_summarize_group_simple_cached_model(self):
+        with patch('microsetta_public_api.repo._taxonomy_repo.TaxonomyRepo.'
+                   'tables', new_callable=PropertyMock) as mock_tables:
+            mock_tables.return_value = {
+                'some-table': {
+                    'table': self.table,
+                    'feature-data-taxonomy': self.taxonomy_df,
+                    'model': TaxonomyModel(self.table, self.taxonomy_df)
+                },
+            }
+            response, code = _summarize_group(self.post_body['sample_ids'],
+                                              "some-table")
+        self.assertEqual(code, 200)
+        exp_keys = ['taxonomy', 'features', 'feature_values',
+                    'feature_variances']
+        obs = json.loads(response)
+
+        self.assertCountEqual(exp_keys, obs.keys())
+        self.assertEqual('((((feature-1,((feature-2)e)d)c)b,'
+                         '(((feature-3)h)g)f)a);',
+                         obs['taxonomy']
+                         )
+        self.assertListEqual(['feature-1', 'feature-2', 'feature-3'],
+                             obs['features'])
+        assert_allclose([1. / 10, 6. / 10, 3. / 10],
+                        obs['feature_values']
+                        )
+        assert_allclose([0, 0, 0],
+                        obs['feature_variances']
+                        )
+
     def test_taxonomy_from_list_summarize_group_simple_with_variances(self):
         with patch('microsetta_public_api.repo._taxonomy_repo.TaxonomyRepo.'
                    'tables', new_callable=PropertyMock) as mock_tables:
diff --git a/microsetta_public_api/api/tests/test_integration.py b/microsetta_public_api/api/tests/test_integration.py
index b94fb55..9eb888a 100644
--- a/microsetta_public_api/api/tests/test_integration.py
+++ b/microsetta_public_api/api/tests/test_integration.py
@@ -125,28 +125,6 @@ class MetadataIntegrationTests(IntegrationTests):
         self.assertCountEqual(['sample_ids'], obs.keys())
         self.assertCountEqual(obs['sample_ids'], exp_ids)
 
-    def test_metadata_filter_on_metric_dne(self):
-        response = self.client.get(
-            # careful not to use a metric that exists in AlphaIntegrationTests
-            '/api/metadata/sample_ids?alpha_metric=bad-metric')
-        self.assertEqual(response.status_code, 404)
-
-    def test_metadata_filter_on_taxonomy_dne(self):
-        response = self.client.get(
-            # careful not to use a table that exists in
-            #  TaxonomyIntegrationTests
-            '/api/metadata/sample_ids?alpha_metric=bad-table')
-        self.assertEqual(response.status_code, 404)
-
-    def test_metadata_filter_on_metric_and_taxonomy_dne(self):
-        response = self.client.get(
-            # careful not to use a metric that exists in AlphaIntegrationTests
-            # careful not to use a table that exists in
-            #  TaxonomyIntegrationTests
-            '/api/metadata/sample_ids?alpha_metric=bad-metric&taxonomy=bad'
-            '-table')
-        self.assertEqual(response.status_code, 404)
-
 
 class TaxonomyIntegrationTests(IntegrationTests):
 
@@ -183,6 +161,7 @@ class TaxonomyIntegrationTests(IntegrationTests):
                                           ['feature-3', 'a; f; g; h', 0.678]],
                                          columns=['Feature ID', 'Taxon',
                                                   'Confidence'])
+
         self.taxonomy2_df.set_index('Feature ID', inplace=True)
 
         self.table3 = biom.Table(np.array([[1, 2],
@@ -232,6 +211,11 @@ class TaxonomyIntegrationTests(IntegrationTests):
                 'table': self.table_biom,
                 'table-format': 'biom',
             },
+            'table-cached-model': {
+                'table': self.table1_filename,
+                'feature-data-taxonomy': self.taxonomy1_filename,
+                'cache-taxonomy': True,
+            },
         }})
         resources.update(config.resources)
 
@@ -242,7 +226,8 @@ class TaxonomyIntegrationTests(IntegrationTests):
         self.assertEqual(response.status_code, 200)
         obs = json.loads(response.data)
         self.assertIn('resources', obs)
-        self.assertCountEqual(['table2', 'table-fish'], obs['resources'])
+        self.assertCountEqual(['table2', 'table-fish', 'table-cached-model'],
+                              obs['resources'])
 
     def test_summarize_group(self):
         response = self.client.post('/api/taxonomy/group/table2',
@@ -268,6 +253,30 @@ class TaxonomyIntegrationTests(IntegrationTests):
                         obs['feature_variances']
                         )
 
+    def test_summarize_group_cached_model(self):
+        response = self.client.post('/api/taxonomy/group/table-cached-model',
+                                    content_type='application/json',
+                                    data=json.dumps({'sample_ids': [
+                                        'sample-1']}))
+
+        self.assertEqual(response.status_code, 200)
+        obs = json.loads(response.data)
+        self.assertCountEqual(['taxonomy', 'features',
+                               'feature_values', 'feature_variances'],
+                              obs.keys())
+
+        self.assertEqual('((((((feature-2)e)d)c)b,(((feature-3)h)g)f)a);',
+                         obs['taxonomy']
+                         )
+        self.assertListEqual(['feature-2', 'feature-3'],
+                             obs['features'])
+        assert_allclose([2. / 5, 3. / 5],
+                        obs['feature_values']
+                        )
+        assert_allclose([0, 0],
+                        obs['feature_variances']
+                        )
+
     def test_summarize_single_sample(self):
         response = self.client.get(
             '/api/taxonomy/single/table2/sample-1',
diff --git a/microsetta_public_api/repo/tests/test_taxonomy_repo.py b/microsetta_public_api/repo/tests/test_taxonomy_repo.py
index 7cb3bcd..f671959 100644
--- a/microsetta_public_api/repo/tests/test_taxonomy_repo.py
+++ b/microsetta_public_api/repo/tests/test_taxonomy_repo.py
@@ -12,6 +12,7 @@ from microsetta_public_api.resources import resources
 from microsetta_public_api.utils.testing import (TempfileTestCase,
                                                  ConfigTestCase)
 from microsetta_public_api.repo._taxonomy_repo import TaxonomyRepo
+from microsetta_public_api.models._taxonomy import Taxonomy as TaxonomyModel
 
 
 class TestTaxonomyRepoHelpers(TestCase):
@@ -113,15 +114,18 @@ class TestTaxonomyRepoWithResources(TempfileTestCase, ConfigTestCase):
             'table2': {
                 'table': self.table1_filename,
                 'feature-data-taxonomy': self.taxonomy1_filename,
+                'cache-taxonomy': False,
             },
             'table3': {
                 'table': self.table3_filename,
                 'feature-data-taxonomy': self.taxonomy2_filename,
+                'cache-taxonomy': False,
                 'variances': self.table3_filename,
             },
             'table4': {
                 'table': self.table_biom,
                 'feature-data-taxonomy': self.taxonomy1_filename,
+                'cache-taxonomy': False,
                 'table-format': 'biom'
             },
             'table5': {
@@ -131,6 +135,11 @@ class TestTaxonomyRepoWithResources(TempfileTestCase, ConfigTestCase):
                 'table': self.table_biom,
                 'table-format': 'biom',
             },
+            'cached-taxonomy-table': {
+                'table': self.table1_filename,
+                'feature-data-taxonomy': self.taxonomy1_filename,
+                'cache-taxonomy': True,
+            },
         }})
         resources.update(config.resources)
         self.repo = TaxonomyRepo()
@@ -140,7 +149,7 @@ class TestTaxonomyRepoWithResources(TempfileTestCase, ConfigTestCase):
         ConfigTestCase.tearDown(self)
 
     def test_available_taxonomy_tables(self):
-        exp = ['table2', 'table3', 'table4']
+        exp = ['table2', 'table3', 'table4', 'cached-taxonomy-table']
         obs = self.repo.resources()
         self.assertCountEqual(exp, obs)
 
@@ -154,6 +163,11 @@ class TestTaxonomyRepoWithResources(TempfileTestCase, ConfigTestCase):
         obs = self.repo.table('table2')
         self.assertEqual(exp, obs)
 
+    def test_get_table_cached_taxonomy(self):
+        exp = self.table
+        obs = self.repo.table('cached-taxonomy-table')
+        self.assertEqual(exp, obs)
+
     def test_get_table_invalid(self):
         with self.assertRaises(ValueError):
             self.repo.table('table1')
@@ -164,6 +178,16 @@ class TestTaxonomyRepoWithResources(TempfileTestCase, ConfigTestCase):
         obs['Confidence'] = obs['Confidence'].astype('float64')
         assert_frame_equal(exp, obs)
 
+    def test_get_taxonomy_model_cached(self):
+        exp = resources['table_resources']['cached-taxonomy-table']['model']
+        obs = self.repo.model('cached-taxonomy-table')
+        self.assertIs(exp, obs)
+
+    def test_get_taxonomy_model_non_cached(self):
+        self.assertNotIn('model', resources['table_resources']['table2'])
+        obs = self.repo.model('table2')
+        self.assertIsInstance(obs, TaxonomyModel)
+
     def test_get_taxonomy_invalid(self):
         with self.assertRaises(ValueError):
             self.repo.table('foo')
diff --git a/microsetta_public_api/tests/test_resources.py b/microsetta_public_api/tests/test_resources.py
index b9f0f8e..ef01b8d 100644
--- a/microsetta_public_api/tests/test_resources.py
+++ b/microsetta_public_api/tests/test_resources.py
@@ -11,6 +11,7 @@ from qiime2.metadata.io import MetadataFileError
 from microsetta_public_api.exceptions import ConfigurationError
 from microsetta_public_api.utils.testing import TempfileTestCase
 from microsetta_public_api.resources import ResourceManager, _parse_q2_data
+from microsetta_public_api.models._taxonomy import Taxonomy as TaxonomyModel
 
 
 class TestResourceManagerUpdateMetadata(TempfileTestCase):
@@ -218,7 +219,12 @@ class TestResourceManagerUpdateTables(TempfileTestCase):
                 'feature-data-taxonomy': self.taxonomy_qza,
                 'variances': self.table_biom,
                 'table-format': 'biom',
-            }
+            },
+            'table-with-cached-taxonomy': {
+                'table': self.table_qza,
+                'feature-data-taxonomy': self.taxonomy_qza,
+                'cache-taxonomy': True,
+            },
         }}
 
     def test_simple_table(self):
@@ -290,7 +296,7 @@ class TestResourceManagerUpdateTables(TempfileTestCase):
                              [subset_table])
         new_table_config = self.resources['table_resources'][
             subset_table]
-        self.assertCountEqual(['table', 'feature-data-taxonomy'],
+        self.assertCountEqual(['table', 'feature-data-taxonomy', 'model'],
                               new_table_config.keys())
 
         exp_table = self.table
@@ -333,7 +339,8 @@ class TestResourceManagerUpdateTables(TempfileTestCase):
                              [subset_table])
         new_table_config = self.resources['table_resources'][
             subset_table]
-        self.assertCountEqual(['table', 'variances', 'feature-data-taxonomy'],
+        self.assertCountEqual(['table', 'variances',
+                               'feature-data-taxonomy', 'model'],
                               new_table_config.keys())
 
         exp_table = self.table
@@ -349,6 +356,26 @@ class TestResourceManagerUpdateTables(TempfileTestCase):
         obs_tax['Confidence'] = obs_tax['Confidence'].astype('float')
         assert_frame_equal(exp_tax, obs_tax)
 
+    def test_table_with_taxonomy_and_cached_taxonomy(self):
+        subset_table = 'table-with-cached-taxonomy'
+        config = {'table_resources': {
+            subset_table: self.config['table_resources'][subset_table]}}
+        self.resources.update(config)
+        self.assertListEqual(list(self.resources['table_resources']),
+                             [subset_table])
+        new_table_config = self.resources['table_resources'][
+            subset_table]
+        self.assertCountEqual(['table', 'feature-data-taxonomy',
+                               'cache-taxonomy', 'model'],
+                              new_table_config.keys())
+
+        exp_table = self.table
+        obs_table = new_table_config['table']
+        assert_frame_equal(exp_table.to_dataframe(dense=True),
+                           obs_table.to_dataframe(dense=True))
+        tax_model = new_table_config.get('model', None)
+        self.assertIsInstance(tax_model, TaxonomyModel)
+
 
 class TestResourceManagerQ2Parse(TempfileTestCase):