From 65ea1b673f1b0e3e4f2ca4854a39d74eb229c3f5 Mon Sep 17 00:00:00 2001 From: Madison Dunitz Date: Mon, 24 Aug 2020 18:26:08 -0500 Subject: [PATCH] Dunitz 1685 hosted annotations (#1789) * save tiledb array to s3, dont cache user annotations * Add option to disable annotation filename prompt (#1787) Co-authored-by: Madison Dunitz * set tiledb default context in cxg_adaptor Co-authored-by: maniarathi Co-authored-by: Severiano Badajoz --- .../src/components/autosave/filenameDialog.js | 1 + client/src/reducers/annotations.js | 4 ++ server/app/app.py | 2 +- server/common/annotations/annotations.py | 12 +--- server/common/annotations/hosted_tiledb.py | 62 ++++++++++++++----- server/common/annotations/local_file_csv.py | 1 + server/data_cxg/cxg_adaptor.py | 8 ++- server/test/__init__.py | 5 +- .../unit/common/test_writable_annotation.py | 7 ++- 9 files changed, 71 insertions(+), 31 deletions(-) diff --git a/client/src/components/autosave/filenameDialog.js b/client/src/components/autosave/filenameDialog.js index ebec1fd1..3937ecee 100644 --- a/client/src/components/autosave/filenameDialog.js +++ b/client/src/components/autosave/filenameDialog.js @@ -101,6 +101,7 @@ class FilenameDialog extends React.Component { const { filenameText } = this.state; return writableCategoriesEnabled && + annotations.promptForFilename && !annotations.dataCollectionNameIsReadOnly && !annotations.dataCollectionName && userinfo.is_authenticated ? ( diff --git a/client/src/reducers/annotations.js b/client/src/reducers/annotations.js index 28dc6533..4949e77b 100644 --- a/client/src/reducers/annotations.js +++ b/client/src/reducers/annotations.js @@ -26,6 +26,7 @@ const Annotations = ( categoryBeingEdited: null, categoryAddingNewLabel: null, labelEditable: { category: null, label: null }, + promptForFilename: true, }, action ) => { @@ -37,10 +38,13 @@ const Annotations = ( action.config.parameters?.[ "annotations-data-collection-name-is-read-only" ] ?? false; + const promptForFilename = + action.config.parameters?.["user_annotation_collection_name_enabled"]; return { ...state, dataCollectionNameIsReadOnly, dataCollectionName, + promptForFilename, }; } diff --git a/server/app/app.py b/server/app/app.py index c84187df..5f47e235 100644 --- a/server/app/app.py +++ b/server/app/app.py @@ -249,7 +249,7 @@ class UserInfoAPI(DatasetResource): class AnnotationsObsAPI(DatasetResource): - @cache_control(public=True, max_age=ONE_WEEK) + @cache_control(public=True, no_store=True) @rest_get_data_adaptor def get(self, data_adaptor): return common_rest.annotations_obs_get(request, data_adaptor) diff --git a/server/common/annotations/annotations.py b/server/common/annotations/annotations.py index 855f19ab..a8443075 100644 --- a/server/common/annotations/annotations.py +++ b/server/common/annotations/annotations.py @@ -64,15 +64,7 @@ class Annotations(metaclass=ABCMeta): """Write the labels (df) to a persistent storage such that it can later be read""" pass + @abstractmethod def update_parameters(self, parameters, data_adaptor): """Update configuration parameters that describe information about the annotations feature""" - params = {} - params["annotations"] = True - - if self.ontology_data: - params["annotations_cell_ontology_enabled"] = True - params["annotations_cell_ontology_terms"] = self.ontology_data - else: - params["annotations_cell_ontology_enabled"] = False - - parameters.update(params) + pass diff --git a/server/common/annotations/hosted_tiledb.py b/server/common/annotations/hosted_tiledb.py index f65a6654..f3df75ed 100644 --- a/server/common/annotations/hosted_tiledb.py +++ b/server/common/annotations/hosted_tiledb.py @@ -11,7 +11,7 @@ from server.common.annotations.annotations import Annotations from server.common.errors import AnnotationCategoryNameError from server.common.utils.sanitization_utils import sanitize_values_in_list from server.common.utils.type_conversion_utils import get_dtypes_and_schemas_of_dataframe, get_dtype_of_array -from server.db.cellxgene_orm import CellxGeneDataset, Annotation +from server.db.cellxgene_orm import Annotation class AnnotationsHostedTileDB(Annotations): @@ -20,7 +20,10 @@ class AnnotationsHostedTileDB(Annotations): def __init__(self, directory_path, db): super().__init__() self.db = db - self.directory_path = directory_path + if directory_path[-1] == "/": + self.directory_path = directory_path + else: + self.directory_path = directory_path + "/" def check_category_names(self, df): original_category_names = df.keys().to_list() @@ -45,25 +48,26 @@ class AnnotationsHostedTileDB(Annotations): def read_labels(self, data_adaptor): user_id = current_app.auth.get_user_id() + if user_id is None: + return dataset_name = data_adaptor.get_location() - dataset_id = str(self.db.query( - table_args=[CellxGeneDataset], - filter_args=[CellxGeneDataset.name == dataset_name] - )[0].id) + dataset_id = self.db.get_or_create_dataset(dataset_name) annotation_object = self.db.query_for_most_recent( Annotation, [Annotation.user_id == user_id, Annotation.dataset_id == dataset_id] ) if annotation_object: df = tiledb.open(annotation_object.tiledb_uri) - pandas_df = self.convert_to_pandas_df(df) + pandas_df = self.convert_to_pandas_df(df, annotation_object.schema_hints) return pandas_df else: return None - def convert_to_pandas_df(self, tileDBArray): + def convert_to_pandas_df(self, tileDBArray, schema_hints): repr_meta = None index_dims = None + schema_hints = json.loads(schema_hints) + if '__pandas_attribute_repr' in tileDBArray.meta: # backwards compatibility... unsure if necessary at this point repr_meta = json.loads(tileDBArray.meta['__pandas_attribute_repr']) @@ -78,7 +82,12 @@ class AnnotationsHostedTileDB(Annotations): if isinstance(col_val[0], bytes): col_val = [value.decode('utf-8') for value in col_val] - if repr_meta and col_name in repr_meta: + if schema_hints and col_name in schema_hints: + type = schema_hints.get(col_name).get("type") + if type and type == "categorical": + new_col = pd.Series(col_val, dtype='category') + data[col_name] = new_col + elif repr_meta and col_name in repr_meta: new_col = pd.Series(col_val, dtype=repr_meta[col_name]) data[col_name] = new_col elif index_dims and col_name in index_dims: @@ -93,20 +102,27 @@ class AnnotationsHostedTileDB(Annotations): return new_df def write_labels(self, df, data_adaptor): - - user_id = current_app.auth.get_user_id() + auth_user_id = current_app.auth.get_user_id() + user_name = current_app.auth.get_user_name() timestamp = time.time() - dataset_name = data_adaptor.get_location() - dataset_id = self.db.get_or_create_dataset(dataset_name) - user_id = self.db.get_or_create_user(user_id) + dataset_location = data_adaptor.get_location() + dataset_id = self.db.get_or_create_dataset(dataset_location) + dataset_name = data_adaptor.get_title() + user_id = self.db.get_or_create_user(auth_user_id) + """ + NOTE: The uri contains the dataset name, user name and a timestamp as a convenience for debugging purposes. + People may have the same name and time.time() can be server dependent. + See - https://docs.python.org/2/library/time.html#time.time - uri = f"{self.directory_path}-{dataset_name}-{user_id}-{timestamp}" + The annotations objects in the database should be used as the source of truth about who an annotation belongs + to (for authorization purposes) and what time it was created (for garbage collection). + """ + uri = f"{self.directory_path}{dataset_name}/{user_name}/{timestamp}" if uri.startswith("s3://"): pass else: os.makedirs(uri, exist_ok=True) _, dataframe_schema_type_hints = get_dtypes_and_schemas_of_dataframe(df) - annotation = Annotation( tiledb_uri=uri, user_id=user_id, @@ -116,9 +132,23 @@ class AnnotationsHostedTileDB(Annotations): if not df.empty: self.check_category_names(df) # convert to tiledb datatypes + for col in df: df[col] = df[col].astype(get_dtype_of_array(df[col])) tiledb.from_pandas(uri, df) self.db.session.add(annotation) self.db.session.commit() + + def update_parameters(self, parameters, data_adaptor): + params = {} + params["annotations"] = True + params["user_annotation_collection_name_enabled"] = False + + if self.ontology_data: + params["annotations_cell_ontology_enabled"] = True + params["annotations_cell_ontology_terms"] = self.ontology_data + else: + params["annotations_cell_ontology_enabled"] = False + + parameters.update(params) diff --git a/server/common/annotations/local_file_csv.py b/server/common/annotations/local_file_csv.py index b463540b..545b8a6e 100644 --- a/server/common/annotations/local_file_csv.py +++ b/server/common/annotations/local_file_csv.py @@ -171,6 +171,7 @@ class AnnotationsLocalFile(Annotations): def update_parameters(self, parameters, data_adaptor): params = {} params["annotations"] = True + params["user_annotation_collection_name_enabled"] = True if self.ontology_data: params["annotations_cell_ontology_enabled"] = True diff --git a/server/data_cxg/cxg_adaptor.py b/server/data_cxg/cxg_adaptor.py index 3963eb68..b739fd6e 100644 --- a/server/data_cxg/cxg_adaptor.py +++ b/server/data_cxg/cxg_adaptor.py @@ -51,8 +51,14 @@ class CxgAdaptor(DataAdaptor): """Set the tiledb context. This should be set before any instances of CxgAdaptor are created""" try: CxgAdaptor.tiledb_ctx = tiledb.Ctx(context_params) + tiledb.default_ctx(context_params) + except tiledb.libtiledb.TileDBError as e: - raise ConfigurationError(f"Invalid tiledb context: {str(e)}") + if e.message == "Global context already initialized!": + if tiledb.default_ctx().config().dict() != CxgAdaptor.tiledb_ctx.config().dict(): + raise ConfigurationError("Cannot change tiledb configuration once it is set") + else: + raise ConfigurationError(f"Invalid tiledb context: {str(e)}") @staticmethod def pre_load_validation(data_locator): diff --git a/server/test/__init__.py b/server/test/__init__.py index 49e28ce5..12159cc4 100644 --- a/server/test/__init__.py +++ b/server/test/__init__.py @@ -33,7 +33,8 @@ def data_with_tmp_tiledb_annotations(ext: MatrixDataType): data_locator = DataLocator(fname) config = AppConfig() config.update_server_config( - multi_dataset__dataroot=data_locator.path, authentication__type="test" + multi_dataset__dataroot=data_locator.path, + authentication__type="test", ) config.update_default_dataset_config( embeddings__names=["umap"], @@ -49,7 +50,7 @@ def data_with_tmp_tiledb_annotations(ext: MatrixDataType): data = MatrixDataLoader(data_locator.abspath()).open(config) annotations = AnnotationsHostedTileDB( tmp_dir, - DbUtils("postgresql://postgres:test_pw@localhost:5432") + DbUtils("postgresql://postgres:test_pw@localhost:5432"), ) return data, tmp_dir, annotations diff --git a/server/test/unit/common/test_writable_annotation.py b/server/test/unit/common/test_writable_annotation.py index a13c7569..f650fecf 100644 --- a/server/test/unit/common/test_writable_annotation.py +++ b/server/test/unit/common/test_writable_annotation.py @@ -21,6 +21,9 @@ class auth(object): def get_user_id(): return "1234" + def get_user_name(): + return "person name" + class WritableTileDBStoredAnnotationTest(unittest.TestCase): def setUp(self): @@ -70,7 +73,7 @@ class WritableTileDBStoredAnnotationTest(unittest.TestCase): self.assertEqual(type(df), tiledb.array.SparseArray) # convert to pandas df - pandas_df = self.annotations.convert_to_pandas_df(df) + pandas_df = self.annotations.convert_to_pandas_df(df, annotation.schema_hints) self.assertEqual(type(pandas_df), pd.DataFrame) def test_write_labels_creates_a_dataset_if_it_doesnt_exist(self): @@ -111,7 +114,9 @@ class WritableTileDBStoredAnnotationTest(unittest.TestCase): self.assertEqual(pandas_df.shape, (self.n_rows, 2)) self.assertEqual(set(pandas_df.columns), {"cat_A", "cat_B"}) + self.assertTrue(self.data.original_obs_index.equals(pandas_df.index)) + self.assertTrue(np.all(pandas_df["cat_A"] == ["label_A"] * self.n_rows)) self.assertTrue(np.all(pandas_df["cat_B"] == ["label_B"] * self.n_rows))