From 4ad9f5875a04da48bd339b78b667b93f2bfd74a7 Mon Sep 17 00:00:00 2001 From: Colin Megill Date: Mon, 17 Aug 2020 11:55:49 -0400 Subject: [PATCH 01/15] xx, yy (#1754) --- client/src/components/scatterplot/scatterplot.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/client/src/components/scatterplot/scatterplot.js b/client/src/components/scatterplot/scatterplot.js index 9b6c2d6a..fa9aa6cb 100644 --- a/client/src/components/scatterplot/scatterplot.js +++ b/client/src/components/scatterplot/scatterplot.js @@ -528,8 +528,8 @@ class Scatterplot extends React.PureComponent { return ( From 298924fef5fef8f1bc2e6134fa69c38437d5ceb5 Mon Sep 17 00:00:00 2001 From: bmccandless Date: Mon, 17 Aug 2020 13:41:03 -0700 Subject: [PATCH 02/15] Separate userinfo from the config endpoint (#1728) * Separate userinfo from the config endpoint previously information about if the user was logged in and their username was part of the config endpoint. However, the config endpoint was previously static, and has a cache control. Rather than not caching the config, a new endpoint called "userinfo" is created to handle that information. The config endpoint still has the non-changing part of the authentication: config: authentication: requires_client_login: True/False login: logout: The userinfo endpoint returns this information: userinfo: is_authenticated: True/False username: if authentication is not enabled then the config does not have an authentication key, and userinfo returns None. Also in the PR are a few minor code improvements and bug fixes Co-authored-by: Colin Megill --- client/src/actions/index.js | 12 ++ .../src/components/autosave/filenameDialog.js | 10 +- client/src/components/categorical/index.js | 33 ++++- client/src/components/menubar/authButtons.js | 8 +- client/src/components/menubar/index.js | 4 +- client/src/reducers/index.js | 2 + client/src/reducers/userinfo.js | 27 ++++ server/app/app.py | 8 ++ server/auth/auth_oauth.py | 31 +++-- server/common/app_config.py | 26 +++- server/common/rest.py | 5 + server/test/unit/auth/test_auth.py | 124 +++++++++--------- 12 files changed, 199 insertions(+), 91 deletions(-) create mode 100644 client/src/reducers/userinfo.js diff --git a/client/src/actions/index.js b/client/src/actions/index.js index a829b45b..4c39f86d 100644 --- a/client/src/actions/index.js +++ b/client/src/actions/index.js @@ -41,6 +41,17 @@ async function configFetch(dispatch) { }); } +async function userInfoFetch(dispatch) { + return fetchJson("userinfo").then((response) => { + const userinfo = { ...response.userinfo }; + dispatch({ + type: "userinfo load complete", + userinfo, + }); + return userinfo; + }); +} + function prefetchEmbeddings(annoMatrix) { /* prefetch requests for all embeddings @@ -62,6 +73,7 @@ const doInitialDataLoad = () => configFetch(dispatch), schemaFetch(dispatch), userColorsFetchAndLoad(dispatch), + userInfoFetch(dispatch), ]); const baseDataUrl = `${globals.API.prefix}${globals.API.version}`; diff --git a/client/src/components/autosave/filenameDialog.js b/client/src/components/autosave/filenameDialog.js index 9b67dc17..ebec1fd1 100644 --- a/client/src/components/autosave/filenameDialog.js +++ b/client/src/components/autosave/filenameDialog.js @@ -14,6 +14,7 @@ import { idhash: state.config?.parameters?.["annotations-user-data-idhash"] ?? null, annotations: state.annotations, auth: state.config?.authentication, + userinfo: state.userinfo, writableCategoriesEnabled: state.config?.parameters?.annotations ?? false, })) class FilenameDialog extends React.Component { @@ -91,13 +92,18 @@ class FilenameDialog extends React.Component { }; render() { - const { writableCategoriesEnabled, annotations, idhash, auth } = this.props; + const { + writableCategoriesEnabled, + annotations, + idhash, + userinfo, + } = this.props; const { filenameText } = this.state; return writableCategoriesEnabled && !annotations.dataCollectionNameIsReadOnly && !annotations.dataCollectionName && - auth.is_authenticated ? ( + userinfo.is_authenticated ? ( - - + + ) : null} ); diff --git a/client/src/components/menubar/authButtons.js b/client/src/components/menubar/authButtons.js index af7a2a93..9323d1e3 100644 --- a/client/src/components/menubar/authButtons.js +++ b/client/src/components/menubar/authButtons.js @@ -4,7 +4,7 @@ import * as globals from "../../globals"; import styles from "./menubar.css"; const Auth = React.memo((props) => { - const { auth } = props; + const { auth, userinfo } = props; if (!auth || (auth && !auth.requires_client_login)) return null; @@ -19,10 +19,10 @@ const Auth = React.memo((props) => { type="button" data-testid="auth-button" disabled={false} - icon={!auth.is_authenticated ? "log-in" : "log-out"} - href={!auth.is_authenticated ? auth.login : auth.logout} + icon={!userinfo["is_authenticated"] ? "log-in" : "log-out"} + href={!userinfo.is_authenticated ? auth.login : auth.logout} > - {!auth.is_authenticated ? "Log In" : "Log Out"} + {!userinfo.is_authenticated ? "Log In" : "Log Out"} diff --git a/client/src/components/menubar/index.js b/client/src/components/menubar/index.js index 1dc8a36a..c06ece70 100644 --- a/client/src/components/menubar/index.js +++ b/client/src/components/menubar/index.js @@ -42,6 +42,7 @@ import { getEmbSubsetView } from "../../util/stateManager/viewStackHelpers"; celllist2: state.differential.celllist2, libraryVersions: state.config?.["library_versions"], auth: state.config?.authentication, + userinfo: state.userinfo, undoDisabled: state["@@undoable/past"].length === 0, redoDisabled: state["@@undoable/future"].length === 0, aboutLink: state.config?.links?.["about-dataset"], @@ -221,6 +222,7 @@ class MenuBar extends React.PureComponent { subsetResetPossible, enableReembedding, auth, + userinfo, } = this.props; const { pendingClipPercentiles } = this.state; @@ -246,7 +248,7 @@ class MenuBar extends React.PureComponent { zIndex: 3, }} > - + { + switch (action.type) { + case "initial data load start": + return { + ...state, + loading: true, + error: null, + }; + case "userinfo load complete": + return { + ...state, + loading: false, + error: null, + ...action.userinfo, + }; + case "initial data load error": + return { + ...state, + error: action.error, + }; + default: + return state; + } +}; + +export default UserInfo; diff --git a/server/app/app.py b/server/app/app.py index d01b1964..06612a1c 100644 --- a/server/app/app.py +++ b/server/app/app.py @@ -240,6 +240,13 @@ class ConfigAPI(DatasetResource): return common_rest.config_get(current_app.app_config, data_adaptor) +class UserInfoAPI(DatasetResource): + @cache_control_always(no_store=True) + @rest_get_data_adaptor + def get(self, data_adaptor): + return common_rest.userinfo_get(current_app.app_config, data_adaptor) + + class AnnotationsObsAPI(DatasetResource): @cache_control(public=True, max_age=ONE_WEEK) @rest_get_data_adaptor @@ -308,6 +315,7 @@ def get_api_resources(bp_api, url_dataroot=None): # Initialization routes add_resource(SchemaAPI, "/schema") add_resource(ConfigAPI, "/config") + add_resource(UserInfoAPI, "/userinfo") # Data routes add_resource(AnnotationsObsAPI, "/annotations/obs") add_resource(AnnotationsVarAPI, "/annotations/var") diff --git a/server/auth/auth_oauth.py b/server/auth/auth_oauth.py index 8ed52036..7f35d43f 100644 --- a/server/auth/auth_oauth.py +++ b/server/auth/auth_oauth.py @@ -1,4 +1,4 @@ -from flask import session, request, redirect, current_app, after_this_request, has_request_context, g +from flask import session, request, redirect, current_app, has_request_context, g from server.auth.auth import AuthTypeClientBase, AuthTypeFactory from server.common.errors import AuthenticationError, ConfigurationError from urllib.parse import urlencode @@ -15,6 +15,7 @@ except ModuleNotFoundError: try: from jose import jwt + from jose.exceptions import ExpiredSignatureError, JWTError, JWTClaimsError except ModuleNotFoundError: missingimport.append("jose") @@ -137,18 +138,15 @@ class AuthTypeOAuth(AuthTypeClientBase): return response def logout(self): + params = {'returnTo' : self.callback_base_url, 'client_id' : self.client_id} + response = redirect(self.client.api_base_url + '/v2/logout?' + urlencode(params)) + if self.session_cookie: if self.CXG_ID_TOKEN in session: del session[self.CXG_ID_TOKEN] else: - @after_this_request - def remove_cookie(response): - response.set_cookie(self.cookie_params["key"], "", expires=0) - self.update_response(response) - return response + response.set_cookie(self.cookie_params["key"], "", expires=0) - params = {'returnTo' : self.callback_base_url, 'client_id' : self.client_id} - response = redirect(self.client.api_base_url + '/v2/logout?' + urlencode(params)) self.update_response(response) return response @@ -178,7 +176,7 @@ class AuthTypeOAuth(AuthTypeClientBase): def get_login_url(self, data_adaptor): """Return the url for the login route""" if current_app.app_config.is_multi_dataset(): - return f"/login?dataset={data_adaptor.uri_path}" + return f"/login?dataset={data_adaptor.uri_path}/" else: return "/login" @@ -205,7 +203,11 @@ class AuthTypeOAuth(AuthTypeClientBase): if token is None: return None - unverified_header = jwt.get_unverified_header(token) + try: + unverified_header = jwt.get_unverified_header(token) + except JWTError: + return None + rsa_key = {} for key in self.jwks['keys']: if key['kid'] == unverified_header['kid']: @@ -227,11 +229,12 @@ class AuthTypeOAuth(AuthTypeClientBase): ) return payload - except jwt.JWTError as e: + except JWTError as e: raise AuthenticationError(f"invalid signature: {str(e)}") - except jwt.ExpiredSignatureError as e: - raise AuthenticationError(f"token expired: {str(e)}") - except jwt.JWTClaimsError as e: + except ExpiredSignatureError: + # TODO, handle expired sessions by refreshing the token + return None + except JWTClaimsError as e: raise AuthenticationError(f"invalid claims {str(e)}") raise AuthenticationError("Unable to find the appropriate key") diff --git a/server/common/app_config.py b/server/common/app_config.py index b3715ecc..e262bc49 100644 --- a/server/common/app_config.py +++ b/server/common/app_config.py @@ -275,10 +275,7 @@ class AppConfig(object): if dataset_config.app__authentication_enable and auth.is_valid_authentication_type(): config["authentication"] = { - "is_authenticated": auth.is_user_authenticated(), "requires_client_login": auth.requires_client_login(), - "username": auth.get_user_name(), - "user_id": auth.get_user_id() } if auth.requires_client_login(): config["authentication"].update({ @@ -288,6 +285,29 @@ class AppConfig(object): return c + def get_client_userinfo(self, data_adaptor): + """ + Return the userinfo as required by the /userinfo REST route + """ + + server_config = self.server_config + dataset_config = data_adaptor.dataset_config + auth = server_config.auth + + # make sure the configuration has been checked. + self.check_config() + + if dataset_config.app__authentication_enable and auth.is_valid_authentication_type(): + userinfo = {} + userinfo["userinfo"] = { + "is_authenticated": auth.is_user_authenticated(), + "username": auth.get_user_name(), + "user_id": auth.get_user_id() + } + return userinfo + else: + return None + class BaseConfig(object): """This class handles the mechanics of updating and checking attributes. diff --git a/server/common/rest.py b/server/common/rest.py index b2e30305..be099f94 100644 --- a/server/common/rest.py +++ b/server/common/rest.py @@ -121,6 +121,11 @@ def config_get(app_config, data_adaptor): return make_response(jsonify(config), HTTPStatus.OK) +def userinfo_get(app_config, data_adaptor): + config = app_config.get_client_userinfo(data_adaptor) + return make_response(jsonify(config), HTTPStatus.OK) + + def annotations_obs_get(request, data_adaptor): fields = request.args.getlist("annotation-name", None) num_columns_requested = len(data_adaptor.get_obs_keys()) if len(fields) == 0 else len(fields) diff --git a/server/test/unit/auth/test_auth.py b/server/test/unit/auth/test_auth.py index 5d6eb748..b07e38d3 100644 --- a/server/test/unit/auth/test_auth.py +++ b/server/test/unit/auth/test_auth.py @@ -21,9 +21,10 @@ class AuthTest(unittest.TestCase): with test_server(app_config=c) as server: session = requests.Session() - r = session.get(f"{server}/d/pbmc3k.cxg/api/v0.2/config") - data_config = r.json() - assert "authentication" not in data_config["config"] + config = session.get(f"{server}/d/pbmc3k.cxg/api/v0.2/config").json() + userinfo = session.get(f"{server}/d/pbmc3k.cxg/api/v0.2/userinfo").json() + self.assertNotIn("authentication", config["config"]) + self.assertIsNone(userinfo) def test_auth_session(self): c = AppConfig() @@ -35,11 +36,12 @@ class AuthTest(unittest.TestCase): with test_server(app_config=c) as server: session = requests.Session() - r = session.get(f"{server}/d/pbmc3k.cxg/api/v0.2/config") - data_config = r.json() - assert data_config["config"]["authentication"]["is_authenticated"] - assert not data_config["config"]["authentication"]["requires_client_login"] - assert data_config["config"]["authentication"]["username"] == "anonymous" + config = session.get(f"{server}/d/pbmc3k.cxg/api/v0.2/config").json() + userinfo = session.get(f"{server}/d/pbmc3k.cxg/api/v0.2/userinfo").json() + + self.assertFalse(config["config"]["authentication"]["requires_client_login"]) + self.assertTrue(userinfo["userinfo"]["is_authenticated"]) + self.assertEqual(userinfo["userinfo"]["username"], "anonymous") def test_auth_test(self): c = AppConfig() @@ -61,45 +63,46 @@ class AuthTest(unittest.TestCase): session = requests.Session() # auth datasets - r = session.get(f"{server}/auth/pbmc3k.cxg/api/v0.2/config") - data_config = r.json() - assert not data_config["config"]["authentication"]["is_authenticated"] - assert data_config["config"]["authentication"]["requires_client_login"] - assert data_config["config"]["authentication"]["username"] is None - assert data_config["config"]["parameters"]["annotations"] + config = session.get(f"{server}/auth/pbmc3k.cxg/api/v0.2/config").json() + userinfo = session.get(f"{server}/auth/pbmc3k.cxg/api/v0.2/userinfo").json() - login_uri = data_config["config"]["authentication"]["login"] - logout_uri = data_config["config"]["authentication"]["logout"] + self.assertFalse(userinfo["userinfo"]["is_authenticated"]) + self.assertIsNone(userinfo["userinfo"]["username"]) + self.assertTrue(config["config"]["authentication"]["requires_client_login"]) + self.assertTrue(config["config"]["parameters"]["annotations"]) - assert login_uri == "/login?dataset=auth/pbmc3k.cxg" - assert logout_uri == "/logout?dataset=auth/pbmc3k.cxg" + login_uri = config["config"]["authentication"]["login"] + logout_uri = config["config"]["authentication"]["logout"] + + self.assertEqual(login_uri, "/login?dataset=auth/pbmc3k.cxg") + self.assertEqual(logout_uri, "/logout?dataset=auth/pbmc3k.cxg") r = session.get(f"{server}/{login_uri}") # check that the login redirect worked - assert r.history[0].status_code == 302 - assert r.url == f"{server}/auth/pbmc3k.cxg/" + self.assertEqual(r.history[0].status_code, 302) + self.assertEqual(r.url, f"{server}/auth/pbmc3k.cxg/") - r = session.get(f"{server}/auth/pbmc3k.cxg/api/v0.2/config") - data_config = r.json() - assert data_config["config"]["authentication"]["is_authenticated"] - assert data_config["config"]["authentication"]["username"] == "test_account" - assert data_config["config"]["parameters"]["annotations"] + config = session.get(f"{server}/auth/pbmc3k.cxg/api/v0.2/config").json() + userinfo = session.get(f"{server}/auth/pbmc3k.cxg/api/v0.2/userinfo").json() + self.assertTrue(userinfo["userinfo"]["is_authenticated"]) + self.assertEqual(userinfo["userinfo"]["username"], "test_account") + self.assertTrue(config["config"]["parameters"]["annotations"]) r = session.get(f"{server}/{logout_uri}") # check that the logout redirect worked - assert r.history[0].status_code == 302 - assert r.url == f"{server}/auth/pbmc3k.cxg/" - r = session.get(f"{server}/auth/pbmc3k.cxg/api/v0.2/config") - data_config = r.json() - assert not data_config["config"]["authentication"]["is_authenticated"] - assert data_config["config"]["authentication"]["username"] is None - assert data_config["config"]["parameters"]["annotations"] + self.assertEqual(r.history[0].status_code, 302) + self.assertEqual(r.url, f"{server}/auth/pbmc3k.cxg/") + config = session.get(f"{server}/auth/pbmc3k.cxg/api/v0.2/config").json() + userinfo = session.get(f"{server}/auth/pbmc3k.cxg/api/v0.2/userinfo").json() + self.assertFalse(userinfo["userinfo"]["is_authenticated"]) + self.assertIsNone(userinfo["userinfo"]["username"]) + self.assertTrue(config["config"]["parameters"]["annotations"]) # no-auth datasets - r = session.get(f"{server}/no-auth/pbmc3k.cxg/api/v0.2/config") - data_config = r.json() - assert "authentication" not in data_config["config"] - assert not data_config["config"]["parameters"]["annotations"] + config = session.get(f"{server}/no-auth/pbmc3k.cxg/api/v0.2/config").json() + userinfo = session.get(f"{server}/no-auth/pbmc3k.cxg/api/v0.2/userinfo").json() + self.assertIsNone(userinfo) + self.assertFalse(config["config"]["parameters"]["annotations"]) def test_auth_test_single(self): c = AppConfig() @@ -111,37 +114,36 @@ class AuthTest(unittest.TestCase): with test_server(app_config=c) as server: session = requests.Session() + config = session.get(f"{server}/api/v0.2/config").json() + userinfo = session.get(f"{server}/api/v0.2/userinfo").json() + self.assertFalse(userinfo["userinfo"]["is_authenticated"]) + self.assertIsNone(userinfo["userinfo"]["username"]) + self.assertTrue(config["config"]["authentication"]["requires_client_login"]) + self.assertTrue(config["config"]["parameters"]["annotations"]) - r = session.get(f"{server}/api/v0.2/config") - data_config = r.json() - assert not data_config["config"]["authentication"]["is_authenticated"] - assert data_config["config"]["authentication"]["requires_client_login"] - assert data_config["config"]["authentication"]["username"] is None - assert data_config["config"]["parameters"]["annotations"] + login_uri = config["config"]["authentication"]["login"] + logout_uri = config["config"]["authentication"]["logout"] - login_uri = data_config["config"]["authentication"]["login"] - logout_uri = data_config["config"]["authentication"]["logout"] - - assert login_uri == "/login" - assert logout_uri == "/logout" + self.assertEqual(login_uri, "/login") + self.assertEqual(logout_uri, "/logout") r = session.get(f"{server}/{login_uri}") # check that the login redirect worked - assert r.history[0].status_code == 302 - assert r.url == f"{server}/" + self.assertEqual(r.history[0].status_code, 302) + self.assertEqual(r.url, f"{server}/") - r = session.get(f"{server}/api/v0.2/config") - data_config = r.json() - assert data_config["config"]["authentication"]["is_authenticated"] - assert data_config["config"]["authentication"]["username"] == "test_account" - assert data_config["config"]["parameters"]["annotations"] + config = session.get(f"{server}/api/v0.2/config").json() + userinfo = session.get(f"{server}/api/v0.2/userinfo").json() + self.assertTrue(userinfo["userinfo"]["is_authenticated"]) + self.assertEqual(userinfo["userinfo"]["username"], "test_account") + self.assertTrue(config["config"]["parameters"]["annotations"]) r = session.get(f"{server}/{logout_uri}") # check that the logout redirect worked - assert r.history[0].status_code == 302 - assert r.url == f"{server}/" - r = session.get(f"{server}/api/v0.2/config") - data_config = r.json() - assert not data_config["config"]["authentication"]["is_authenticated"] - assert data_config["config"]["authentication"]["username"] is None - assert data_config["config"]["parameters"]["annotations"] + self.assertEqual(r.history[0].status_code, 302) + self.assertEqual(r.url, f"{server}/") + config = session.get(f"{server}/api/v0.2/config").json() + userinfo = session.get(f"{server}/api/v0.2/userinfo").json() + self.assertFalse(userinfo["userinfo"]["is_authenticated"]) + self.assertIsNone(userinfo["userinfo"]["username"]) + self.assertTrue(config["config"]["parameters"]["annotations"]) From 1acb8e4a6f13cdef37f8e777b8784d482f4005c1 Mon Sep 17 00:00:00 2001 From: Severiano Badajoz Date: Mon, 17 Aug 2020 16:51:58 -0700 Subject: [PATCH 03/15] Remove support for non-chromium Edge (#1761) * bump browserlist Edge to 79+ * bump edge version on unsupported browser page --- client/configuration/webpack/obsoleteHTMLTemplate.html | 2 +- client/package.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/client/configuration/webpack/obsoleteHTMLTemplate.html b/client/configuration/webpack/obsoleteHTMLTemplate.html index 500c7db5..ddb2dd27 100644 --- a/client/configuration/webpack/obsoleteHTMLTemplate.html +++ b/client/configuration/webpack/obsoleteHTMLTemplate.html @@ -75,7 +75,7 @@ src="https://cellxgene.cziscience.com/s3/cellxgene/static/images/edge.png" style="width: 80px; height: 80px;" /> -
Edge ≥ 15
+
Edge ≥ 79
diff --git a/client/package.json b/client/package.json index b49053d0..aa4892f9 100644 --- a/client/package.json +++ b/client/package.json @@ -34,7 +34,7 @@ "Safari >= 10.1", "iOS >= 10.3", "Firefox >= 60", - "Edge >= 15", + "Edge >= 79", "not Explorer > 0" ], "dependencies": { From 994c20c0945ed0ede098e67df5465e7512dce584 Mon Sep 17 00:00:00 2001 From: maniarathi Date: Mon, 17 Aug 2020 17:28:29 -0700 Subject: [PATCH 04/15] Move cxgtool into CLI and modularize conversion functions (#1701) --- server/cli/convert_to_cxg.py | 131 ++++ server/common/annotations/hosted_tiledb.py | 17 +- server/common/app_config.py | 2 +- server/common/corpora.py | 20 +- server/common/utils/corpora_constants.py | 16 + server/common/utils/cxg_constants.py | 4 + server/common/utils/cxg_generation_utils.py | 179 +++++ server/common/utils/type_conversion_utils.py | 11 + server/converters/cxgtool.py | 669 ------------------ server/converters/h5ad_data_file.py | 250 +++++++ server/test/unit/common/test_api.py | 6 +- server/test/unit/common/test_corpora.py | 9 +- .../common/utils/test_cxg_generation_utils.py | 149 ++++ .../utils/test_type_conversion_utils.py | 18 +- server/test/unit/compute/test_diffexp_cxg.py | 33 +- server/test/unit/converters/test_cxgtool.py | 41 -- .../unit/converters/test_h5ad_data_file.py | 235 ++++++ 17 files changed, 1032 insertions(+), 758 deletions(-) create mode 100644 server/cli/convert_to_cxg.py create mode 100644 server/common/utils/corpora_constants.py create mode 100644 server/common/utils/cxg_constants.py create mode 100644 server/common/utils/cxg_generation_utils.py delete mode 100644 server/converters/cxgtool.py create mode 100644 server/converters/h5ad_data_file.py create mode 100644 server/test/unit/common/utils/test_cxg_generation_utils.py delete mode 100644 server/test/unit/converters/test_cxgtool.py create mode 100644 server/test/unit/converters/test_h5ad_data_file.py diff --git a/server/cli/convert_to_cxg.py b/server/cli/convert_to_cxg.py new file mode 100644 index 00000000..e8d37936 --- /dev/null +++ b/server/cli/convert_to_cxg.py @@ -0,0 +1,131 @@ +from os import path + +import click + +from server.converters.h5ad_data_file import H5ADDataFile + + +@click.command( + name="convert", + short_help="Converts an H5AD dataset to the CXG format.", + help="Converts an H5AD dataset to the CXG format. The CXG format is a cellxgene-private data format " + "that has performance and access characteristics amenable to a multi-dataset, multi-user serving " + "environment. You will be able to launch the cellxgene using the `cellxgene launch` command as " + "usually with the generated CXG file.", +) +@click.argument( + "input-file", + nargs=1, + help="Path to the H5AD input file to be converted.", + type=click.Path(exists=True, dir_okay=False), +) +@click.option( + "-o", + "--output-dir", + help="Name of the output CXG directory. If not provided, will default to be the input filename with a " + "CXG extension.", +) +@click.option( + "-b", + "--backed", + help="When true, loads the H5AD in file backed mode. This will cause the conversion to be slower, " + "but will use less memory.", + default=False, + show_default=True, + is_flag=True, +) +@click.option( + "-t", + "--title", + help="Human readable dataset title that will be included as metadata about the CXG file. If omitted, " + "the dataset title will be the filename.", +) +@click.option( + "-a", + "--about", + help="A fully qualified URL that provides more information about the dataset and will be included as " + "metadata about the CXG file.", +) +@click.option( + "-s", + "--sparse-threshold", + help="If the dataset's percent of non-zero values falls belows the specified threshold, then the X " + "array of the dataset will be sparse. Since the default value is 0.0, the default will be to " + "convert to dense array.", + default=0.0, + show_default=True, +) +@click.option("--obs-names", + help="Name to a column in the obs dataframe that will be used as the index for the dataframe instead of " + "the one designated by the dataframe generated-index.") +@click.option("--var-names", + help="Name to a column in the var dataframe that will be used as the index for the dataframe instead of " + "the one designated by the dataframe generated-index.") +@click.option( + "--disable-custom-colors", + help="When set, conversion process will not extract scanpy-compatible category colors from the H5AD file.", + default=False, + show_default=True, + is_flag=True, +) +@click.option( + "--disable-corpora-schema", + "When set, conversion process will neither extract nor store Corpora schema information. See " + "https://github.com/chanzuckerberg/corpora-data-portal/blob/main/backend/schema/corpora_schema.md for " + "more information.", + default=False, + show_default=True, + is_flag=True, +) +@click.option( + "--overwrite", + help="When set to true, will overwrite the output file if the output file already exists.", + default=False, + show_default=True, + is_flag=True, +) +@click.option("-v", "--verbose", count=True) +@click.help_option("--help", "-h", help="Show this message and exit.") +def convert_to_cxg( + input_file, + output_directory, + backed, + title, + about, + sparse_threshold, + obs_names, + var_names, + disable_custom_colors, + disable_corpora_schema, + should_overwrite, +): + """ + Convert a dataset file into CXG. + """ + + h5ad_data_file = H5ADDataFile(input_file, backed, title, about, obs_names, var_names, + use_corpora_schema=not disable_corpora_schema) + + # Get the directory that will hold all the CXG files + cxg_output_container = get_output_directory(input_file, output_directory, should_overwrite) + + h5ad_data_file.to_cxg(cxg_output_container, sparse_threshold, + convert_anndata_colors_to_cxg_colors=not disable_custom_colors) + + +def get_output_directory(input_filename, output_directory, should_overwrite): + """ + Get the name of the CXG output directory to be created/populated during the dataset conversion. + """ + + if not path.isdir(output_directory) or (path.isdir(output_directory) and should_overwrite): + if output_directory.endswith(".cxg"): + return output_directory + return output_directory + ".cxg" + if path.isdir(output_directory) and not should_overwrite: + raise click.BadParameter( + f"Output directory {output_directory} already exists. If you'd like to overwrite, then run the command " + f"with the --overwrite flag." + ) + + return path.splitext(input_filename)[1] + ".cxg" diff --git a/server/common/annotations/hosted_tiledb.py b/server/common/annotations/hosted_tiledb.py index 05b017a4..16ac7b29 100644 --- a/server/common/annotations/hosted_tiledb.py +++ b/server/common/annotations/hosted_tiledb.py @@ -8,7 +8,9 @@ import tiledb from flask import current_app from server.common.annotations.annotations import Annotations -from server.converters.cxgtool import sanitize_keys, generate_schema_hints_and_convert_value_types, cxg_dtype +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 @@ -21,7 +23,12 @@ class AnnotationsHostedTileDB(Annotations): self.directory_path = directory_path def check_category_names(self, df): - sanitize_keys(df.keys().to_list(), False) + original_category_names = df.keys().to_list() + sanitized_category_names = set(sanitize_values_in_list(original_category_names).values()) + unsanitary_original_category_names = set(original_category_names).difference(sanitized_category_names) + if unsanitary_original_category_names: + raise AnnotationCategoryNameError( + f"{unsanitary_original_category_names} are not valid category names, please resubmit") def is_safe_collection_name(self, name): """ @@ -94,19 +101,19 @@ class AnnotationsHostedTileDB(Annotations): pass else: os.makedirs(uri, exist_ok=True) - schema_hints, values = generate_schema_hints_and_convert_value_types(df) + _, dataframe_schema_type_hints = get_dtypes_and_schemas_of_dataframe(df) annotation = Annotation( tiledb_uri=uri, user_id=user_id, dataset_id=str(dataset_id), - schema_hints=json.dumps(schema_hints) + schema_hints=json.dumps(dataframe_schema_type_hints) ) if not df.empty: self.check_category_names(df) # convert to tiledb datatypes for col in df: - df[col] = df[col].astype(cxg_dtype(df[col])) + df[col] = df[col].astype(get_dtype_of_array(df[col])) tiledb.from_pandas(uri, df) self.db.session.add(annotation) diff --git a/server/common/app_config.py b/server/common/app_config.py index e262bc49..df7a0f96 100644 --- a/server/common/app_config.py +++ b/server/common/app_config.py @@ -768,7 +768,7 @@ class DatasetConfig(BaseConfig): self.user_annotations__ontology__obo_location = dc["user_annotations"]["ontology"]["obo_location"] self.user_annotations__hosted_tiledb_array__db_uri = dc["user_annotations"]["hosted_tiledb_array"]["db_uri"] self.user_annotations__hosted_tiledb_array__hosted_file_directory = \ - dc["user_annotations"]["hosted_tiledb_array"]["hosted_file_directory"] # noqa E501 + dc["user_annotations"]["hosted_tiledb_array"]["hosted_file_directory"] # noqa E501 self.embeddings__names = dc["embeddings"]["names"] self.embeddings__enable_reembedding = dc["embeddings"]["enable_reembedding"] diff --git a/server/common/corpora.py b/server/common/corpora.py index 9f48873b..d616f75b 100644 --- a/server/common/corpora.py +++ b/server/common/corpora.py @@ -9,6 +9,7 @@ import collections import json from server.cli.upgrade import validate_version_str +from server.common.utils.corpora_constants import CorporaConstants def corpora_get_versions_from_anndata(adata): @@ -56,26 +57,13 @@ def corpora_get_props_from_anndata(adata): if not version_is_supported: raise ValueError("Unsupported Corpora schema version") - required_simple_fields = [ - "version", - "title", - "layer_descriptions", - "organism", - "organism_ontology_term_id", - "project_name", - "project_description", - ] - # Spec says some values encoded as JSON due to the inability of AnnData to store complex types. - required_json_fields = ["contributors", "project_links"] - optional_simple_fields = ["preprint_doi", "publication_doi", "default_embedding", "default_field", "tags"] - corpora_props = {} - for key in required_simple_fields: + for key in CorporaConstants.REQUIRED_SIMPLE_METADATA_FIELDS: if key not in adata.uns: raise KeyError(f"missing Corpora schema field {key}") corpora_props[key] = adata.uns[key] - for key in required_json_fields: + for key in CorporaConstants.REQUIRED_JSON_ENCODED_METADATA_FIELD: if key not in adata.uns: raise KeyError(f"missing Corpora schema field {key}") try: @@ -83,7 +71,7 @@ def corpora_get_props_from_anndata(adata): except json.JSONDecodeError: raise json.JSONDecodeError(f"Corpora schema field {key} is expected to be a valid JSON string") - for key in optional_simple_fields: + for key in CorporaConstants.OPTIONAL_SIMPLE_METADATA_FIELDS: if key in adata.uns: corpora_props[key] = adata.uns[key] diff --git a/server/common/utils/corpora_constants.py b/server/common/utils/corpora_constants.py new file mode 100644 index 00000000..c0c98168 --- /dev/null +++ b/server/common/utils/corpora_constants.py @@ -0,0 +1,16 @@ +class CorporaConstants(object): + REQUIRED_SIMPLE_METADATA_FIELDS = [ + "version", + "title", + "layer_descriptions", + "organism", + "organism_ontology_term_id", + "project_name", + "project_description", + ] + + # The Corpora specification requires some values encoded as JSON due to the inability of AnnData to store complex + # types. + REQUIRED_JSON_ENCODED_METADATA_FIELD = ["contributors", "project_links"] + + OPTIONAL_SIMPLE_METADATA_FIELDS = ["preprint_doi", "publication_doi", "default_embedding", "default_field", "tags"] diff --git a/server/common/utils/cxg_constants.py b/server/common/utils/cxg_constants.py new file mode 100644 index 00000000..5982ec15 --- /dev/null +++ b/server/common/utils/cxg_constants.py @@ -0,0 +1,4 @@ +class CxgConstants(object): + # The CXG container version number. Must be a semver string (major.minor.patch) + # DO NOT UPDATE THIS WITHOUT ALSO UPDATING CXG SPECIFICATION. + CXG_VERSION = "0.2.0" diff --git a/server/common/utils/cxg_generation_utils.py b/server/common/utils/cxg_generation_utils.py new file mode 100644 index 00000000..f336c0ae --- /dev/null +++ b/server/common/utils/cxg_generation_utils.py @@ -0,0 +1,179 @@ +import json + +import numpy as np +import tiledb + +from server.common.utils.type_conversion_utils import get_dtype_of_array, get_dtype_and_schema_of_array + + +def convert_dictionary_to_cxg_group(cxg_container, metadata_dict, group_metadata_name="cxg_group_metadata"): + """ + Saves the contents of the dictionary to the CXG output directory specified. + + This function is primarily used to save metadata about a dataset to the CXG directory. At some point, tiledb will + have support for metadata on groups at which point the utility of this function should be revisited. Until such + feature exists, this function create an empty array and annotate that array. + + For more information, visit https://github.com/TileDB-Inc/TileDB-Py/issues/254. + """ + + array_name = f"{cxg_container}/{group_metadata_name}" + + # Because TileDB does not allow one to attach metadata directly to a CXG group, we need to have a workaround + # where we create an empty array and attached the metadata onto to this empty array. Below we construct this empty + # array. + tiledb.from_numpy(array_name, np.zeros((1,))) + + with tiledb.DenseArray(array_name, mode="w") as metadata_array: + for key, value in metadata_dict.items(): + metadata_array.meta[key] = value + + +def convert_dataframe_to_cxg_array(cxg_container, dataframe_name, dataframe, index_column_name, ctx): + """ + Saves the contents of the dataframe to the CXG output directory specified. + + Current access patterns are oriented toward reading very large slices of the dataframe, one attribute at a time. + Attribute data also tends to be (often) repetitive (bools, categories, strings). Given this, we use a large tile + size (1000) and very aggressive compression levels. + """ + + def create_dataframe_array(array_name, dataframe): + tiledb_filter = tiledb.FilterList( + [ + # Attempt aggressive compression as many of these dataframes are very repetitive strings, bools and + # other non-float data. + tiledb.ZstdFilter(level=22), + ] + ) + attrs = [ + tiledb.Attr(name=column, dtype=get_dtype_of_array(dataframe[column]), filters=tiledb_filter) + for column in dataframe + ] + domain = tiledb.Domain( + tiledb.Dim(domain=(0, dataframe.shape[0] - 1), tile=min(dataframe.shape[0], 1000), dtype=np.uint32) + ) + schema = tiledb.ArraySchema( + domain=domain, sparse=False, attrs=attrs, cell_order="row-major", tile_order="row-major" + ) + tiledb.DenseArray.create(array_name, schema) + + array_name = f"{cxg_container}/{dataframe_name}" + + create_dataframe_array(array_name, dataframe) + + with tiledb.DenseArray(array_name, mode="w", ctx=ctx) as array: + value = {} + schema_hints = {} + for column_name, column_values in dataframe.items(): + dtype, hints = get_dtype_and_schema_of_array(column_values) + + value[column_name] = column_values.to_numpy(dtype=dtype) + if hints: + schema_hints.update({column_name: hints}) + + schema_hints.update({"index": index_column_name}) + array[:] = value + array.meta["cxg_schema"] = json.dumps(schema_hints) + + tiledb.consolidate(array_name, ctx=ctx) + + +def convert_ndarray_to_cxg_dense_array(ndarray_name, ndarray, ctx): + """ + Saves contents of ndarray to the CXG output directory specified. + + Generally this function is used to convert dataset embeddings. Because embeddings are typically accessed with + very large slices (or all of the embedding), they do not benefit from overly aggressive compression due to their + format. Given this, we use a large tile size (1000) but only default compression level. + """ + + def create_ndarray_array(ndarray_name, ndarray): + filters = tiledb.FilterList([tiledb.ZstdFilter()]) + attrs = [tiledb.Attr(dtype=ndarray.dtype, filters=filters)] + dimensions = [ + tiledb.Dim( + domain=(0, ndarray.shape[dimension] - 1), tile=min(ndarray.shape[dimension], 1000), dtype=np.uint32 + ) + for dimension in range(ndarray.ndim) + ] + domain = tiledb.Domain(*dimensions) + schema = tiledb.ArraySchema( + domain=domain, sparse=False, attrs=attrs, capacity=1_000_000, cell_order="row-major", tile_order="row-major" + ) + tiledb.DenseArray.create(ndarray_name, schema) + + create_ndarray_array(ndarray_name, ndarray) + + with tiledb.DenseArray(ndarray_name, mode="w", ctx=ctx) as array: + array[:] = ndarray + + tiledb.consolidate(ndarray_name, ctx=ctx) + + +def convert_matrix_to_cxg_array( + matrix_name, matrix, encode_as_sparse_array, ctx, column_shift_for_sparse_encoding=None +): + """ + Converts a numpy array matrix into a TileDB SparseArray of DenseArray based on whether `encode_as_sparse_array` + is true or not. Note that when the matrix is encoded as a SparseArray, it only writes the values that are + nonzero. This means that if you count the number of elements in the SparseArray, it will not equal the total + number of elements in the matrix, only the number of nonzero elements. + + Furthermore, if the `column_shift_for_sparse_encoding` matrix is not None, this function will subtract the sparse + encoding from the original given matrix and as previously stated, only write the nonzero values to the TileDB + SparseArray. + """ + + def create_matrix_array(matrix_name, number_of_rows, number_of_columns, encode_as_sparse_array): + filters = tiledb.FilterList([tiledb.ZstdFilter()]) + attrs = [tiledb.Attr(dtype=np.float32, filters=filters)] + if encode_as_sparse_array: + domain = tiledb.Domain( + tiledb.Dim(name="obs", domain=(0, number_of_rows - 1), tile=min(number_of_rows, 512), dtype=np.uint32), + tiledb.Dim( + name="var", domain=(0, number_of_columns - 1), tile=min(number_of_columns, 2048), dtype=np.uint32 + ), + ) + else: + domain = tiledb.Domain( + tiledb.Dim(name="obs", domain=(0, number_of_rows - 1), tile=min(number_of_rows, 50), dtype=np.uint32), + tiledb.Dim( + name="var", domain=(0, number_of_columns - 1), tile=min(number_of_columns, 100), dtype=np.uint32 + ), + ) + schema = tiledb.ArraySchema( + domain=domain, sparse=encode_as_sparse_array, attrs=attrs, cell_order="row-major", tile_order="col-major" + ) + if encode_as_sparse_array: + tiledb.SparseArray.create(matrix_name, schema) + else: + tiledb.DenseArray.create(matrix_name, schema) + + number_of_rows = matrix.shape[0] + number_of_columns = matrix.shape[1] + stride = min(int(np.power(10, np.around(np.log10(1e9 / number_of_columns)))), 10_000) + + create_matrix_array(matrix_name, number_of_rows, number_of_columns, encode_as_sparse_array) + + if encode_as_sparse_array: + with tiledb.SparseArray(matrix_name, mode="w", ctx=ctx) as array: + for start_row_index in range(0, number_of_rows, stride): + end_row_index = min(start_row_index + stride, number_of_rows) + matrix_subset = matrix[start_row_index:end_row_index, :] + if not isinstance(matrix_subset, np.ndarray): + matrix_subset = matrix_subset.toarray() + if column_shift_for_sparse_encoding is not None: + matrix_subset = matrix_subset - column_shift_for_sparse_encoding + indices = np.nonzero(matrix_subset) + trow = indices[0] + start_row_index + array[trow, indices[1]] = matrix_subset[indices[0], indices[1]] + + else: + with tiledb.DenseArray(matrix_name, mode="w", ctx=ctx) as array: + for start_row_index in range(0, number_of_rows, stride): + end_row_index = min(start_row_index + stride, number_of_rows) + matrix_subset = matrix[start_row_index:end_row_index, :] + if not isinstance(matrix_subset, np.ndarray): + matrix_subset = matrix_subset.toarray() + array[start_row_index:end_row_index, :] = matrix_subset diff --git a/server/common/utils/type_conversion_utils.py b/server/common/utils/type_conversion_utils.py index 8b467b47..eca8b938 100644 --- a/server/common/utils/type_conversion_utils.py +++ b/server/common/utils/type_conversion_utils.py @@ -4,6 +4,17 @@ import numpy as np import pandas as pd +def get_dtypes_and_schemas_of_dataframe(dataframe: pd.DataFrame): + dtypes_by_column_name = {} + schema_type_hints_by_column_name = {} + + for column_name, column_values in dataframe.items(): + dtypes_by_column_name[column_name], schema_type_hints_by_column_name[column_name] = \ + get_dtype_and_schema_of_array(column_values) + + return dtypes_by_column_name, schema_type_hints_by_column_name + + def get_dtype_of_array(array: pd.Series): return get_dtype_and_schema_of_array(array)[0] diff --git a/server/converters/cxgtool.py b/server/converters/cxgtool.py deleted file mode 100644 index 5116c4e6..00000000 --- a/server/converters/cxgtool.py +++ /dev/null @@ -1,669 +0,0 @@ -""" -This program converts an [AnnData H5AD](https://anndata.readthedocs.io/en/stable/) -into a cellxgene TileDB structure, aka a [CXG](../../dev_docs/cxg.md). - -IF YOU UPDATE THIS FILE, IN ANY WAY THAT MODIFIES THE CXG FORMAT or CONTENTS, -YOU MUST UPDATE THE CXG SPECIFICATION and VERSION NUMBER. -""" -import re -import anndata -import tiledb -import argparse -import numpy as np -from os.path import splitext, basename -import json -from scipy.stats import mode - -from server.common.colors import convert_anndata_category_colors_to_cxg_category_colors -from server.common.errors import ColorFormatException, AnnotationCategoryNameError -from server.common.corpora import ( - corpora_get_props_from_anndata, - corpora_get_versions_from_anndata, - corpora_is_version_supported, -) - - -# the CXG container version number. Must be a semver string (major.minor.patch) -# DO NOT UPDATE THIS WITHOUT ALSO UPDATING THE CXG SPECIFICATION. -CXG_VERSION = "0.2.0" - -# log_level must have a default -log_level = 3 - - -def log(level, *args): - global log_level - if log_level and level <= log_level: - print(*args) - - -def main(): - parser = argparse.ArgumentParser() - parser.add_argument("h5ad", nargs="?", help="H5AD file name") - parser.add_argument( - "--backed", action="store_true", help="loaded in file backed mode. Will be slower, but use less memory." - ) - parser.add_argument( - "--disable-custom-colors", - action="store_true", - default=False, - help="Do not extract scanpy-compatible category colors from h5ad file.", - ) - parser.add_argument( - "--obs-names", help="Name of annotation to use for observations. If not specified, will use the obs index." - ) - parser.add_argument( - "--var-names", help="Name of annotation to use for variables. If not specified, will use the var index." - ) - parser.add_argument("--verbose", "-v", action="count", default=0, help="verbose output") - parser.add_argument("--title", help="Human readable dataset title. If omitted, will use filename") - parser.add_argument( - "--about", - metavar="", - help="URL providing more information about the dataset (hint: must be a fully specified absolute URL).", - ) - parser.add_argument("--out", "--output", "-o", help="output CXG file name") - parser.add_argument( - "--sparse-threshold", - "-s", - type=float, - default=0.0, # force dense by default - help="The X array will be sparse if the percent of non-zeros falls below this value", - ) - parser.add_argument( - "--disable-corpora", - action="store_true", - default=False, - help="Disable extraction and storing of Corpora schema information.", - ) - args = parser.parse_args() - - global log_level - log_level = args.verbose - - adata = anndata.read_h5ad(args.h5ad, backed="r" if args.backed else None) - log(1, f"{basename(args.h5ad)} loaded...") - - basefname = splitext(basename(args.h5ad))[0] - out = args.out if args.out is not None else basefname - container = out if splitext(out)[1] == ".cxg" else out + ".cxg" - - corpora_props = load_corpora_props(args, adata) if not args.disable_corpora else None - cxg_group_metadata = create_cxg_group_metadata( - adata, - basefname, - title=args.title, - about=args.about, - corpora_props=corpora_props, - extract_colors=not args.disable_custom_colors, - ) - - write_cxg( - adata, - container, - cxg_group_metadata=cxg_group_metadata, - var_names=args.var_names, - obs_names=args.obs_names, - sparse_threshold=args.sparse_threshold, - ) - - log(1, "done") - - -def write_cxg(adata, container, cxg_group_metadata, var_names=None, obs_names=None, sparse_threshold=5.0): - if not adata.var.index.is_unique: - raise ValueError("Variable index is not unique - unable to convert.") - if not adata.obs.index.is_unique: - raise ValueError("Observation index is not unique - unable to convert.") - - """ - TileDB bug TileDB-Inc/TileDB#1575 requires that we sanitize all column names - prior to saving. This can be reverted when the bug is fixed. - """ - log(0, "Warning: sanitizing all dataframe column names.") - clean_all_column_names(adata) - - ctx = tiledb.Ctx( - { - "sm.num_reader_threads": 32, - "sm.num_writer_threads": 32, - "sm.consolidation.buffer_size": 1 * 1024 * 1024 * 1024, - } - ) - - tiledb.group_create(container, ctx=ctx) - log(1, f"\t...group created, with name {container}") - - # dataset metadata - save_metadata(container, cxg_group_metadata) - log(1, "\t...dataset metadata saved") - - # var/gene dataframe - save_dataframe(container, "var", adata.var, var_names, ctx=ctx) - log(1, "\t...var dataframe created") - - # obs/cell dataframe - save_dataframe(container, "obs", adata.obs, obs_names, ctx=ctx) - log(1, "\t...obs dataframe created") - - # embeddings - e_container = f"{container}/emb" - tiledb.group_create(e_container, ctx=ctx) - save_embeddings(e_container, adata, ctx) - log(1, "\t...embeddings created") - - # X matrix - save_X(container, adata.X, ctx, sparse_threshold) - log(1, "\t...X created") - - -""" -TODO: the code used to handle type inferencing should not be duplicated between -this tool and the server/common/utils code. When this tool is merged into -the cellxgene CLI, consolidate. -""" - - -def dtype_to_schema(dtype): - if dtype == np.float32: - return (np.float32, {}) - elif dtype == np.int32: - return (np.int32, {}) - elif dtype == np.bool_: - return (np.uint8, {"type": "boolean"}) - elif dtype == np.str: - return (np.unicode, {"type": "string"}) - elif dtype == "category": - typ, hint = cxg_type(dtype.categories) - return (typ, {"type": "categorical", "categories": dtype.categories.tolist()}) - else: - raise TypeError(f"Annotations of type {dtype} are unsupported.") - - -def _can_cast_to_float32(array): - if array.dtype.kind == "f": - # force downcast for all floats - return True - return False - - -def _can_cast_to_int32(array): - if array.dtype.kind in ["i", "u"]: - if np.can_cast(array.dtype, np.int32): - return True - ii32 = np.iinfo(np.int32) - if array.min() >= ii32.min and array.max() <= ii32.max: - return True - return False - - -def cxg_type(array): - try: - return dtype_to_schema(array.dtype) - except TypeError: - dtype = array.dtype - data_kind = dtype.kind - if _can_cast_to_float32(array): - return (np.float32, {}) - elif _can_cast_to_int32(array): - return (np.int32, {}) - elif data_kind == "O" and dtype == "object": - return (np.unicode, {"type": "string"}) - else: - raise TypeError(f"Annotations of type {dtype} are unsupported.") - - -def cxg_dtype(array): - return cxg_type(array)[0] - - -def create_dataframe(name, df, ctx): - """ - Current access patterns are oriented toward reading very large slices of - the dataframe, one attribute at a time. Attribute data also tends to be - (often) repetitive (bools, categories, strings). - Given this, we use: - * a large tile size (1000) - * very aggressive compression levels - """ - filter = tiledb.FilterList( - [ - # attempt aggressive compression as many of these dataframes are very repetitive - # strings, bools and other non-float data. - tiledb.ZstdFilter(level=22), - ] - ) - attrs = [tiledb.Attr(name=col, dtype=cxg_dtype(df[col]), filters=filter) for col in df] - domain = tiledb.Domain(tiledb.Dim(domain=(0, df.shape[0] - 1), tile=min(df.shape[0], 1000), dtype=np.uint32)) - schema = tiledb.ArraySchema( - domain=domain, sparse=False, attrs=attrs, cell_order="row-major", tile_order="row-major" - ) - tiledb.DenseArray.create(name, schema) - - -def create_unique_column_name(df_cols, col_name_prefix): - """ - given the columns of a dataframe, and a name prefix, return a column name which - does not exist in the dataframe, AND which is prefixed by `prefix` - - The approach is to append a numeric suffix, starting at zero and increasing by - one, until an unused name is found (eg, prefix_0, prefix_1, ...). - """ - suffix = 0 - while f"{col_name_prefix}{suffix}" in df_cols: - suffix += 1 - return f"{col_name_prefix}{suffix}" - - -def alias_index_col(df, df_name, index_col_name): - """ - We rely in the existance of a unique, human-readable index for - any dataframe (eg, var is typically gene name, obs the cell name). - The user can specify these via the --obs-names and --var-names config. - If they are not specified, use the existing index to create them, giving - the resulting column a unique name (eg, "name"). - - In both cases, enforce that the result is unique, and communicate the - index column name via the 'index' field in the schema hints. - """ - if index_col_name is None: - if not df.index.is_unique: - raise KeyError( - f"Values in {df_name}.index must be unique. " - "Please prepare data to contain unique index values, or specify an " - "alternative with --{ax_name}-name." - ) - index_col_name = create_unique_column_name(df.columns, "name_") - # turn the index into a normal column - df.rename_axis(index_col_name, inplace=True) - df.reset_index(inplace=True) - - elif index_col_name in df.columns: - # User has specified alternative column for unique names, and it exists - if not df[index_col_name].is_unique: - raise KeyError( - f"Values in {df_name}.{index_col_name} must be unique. Please prepare data to contain unique values." - ) - - else: - raise KeyError(f"Annotation {index_col_name}, specified in --{df_name}-name, does not exist.") - - return (df, index_col_name) - - -def generate_schema_hints_and_convert_value_types(df): - value = {} - schema_hints = {} - for k, v in df.items(): - dtype, hints = cxg_type(v) - value[k] = v.to_numpy(dtype=dtype) - if hints: - schema_hints.update({k: hints}) - return schema_hints, value - - -def save_dataframe(container, name, df, index_col_name, ctx): - A_name = f"{container}/{name}" - (df, index_col_name) = alias_index_col(df, name, index_col_name) - create_dataframe(A_name, df, ctx=ctx) - with tiledb.DenseArray(A_name, mode="w", ctx=ctx) as A: - schema_hints, value = generate_schema_hints_and_convert_value_types(df) - schema_hints.update({"index": index_col_name}) - # convert all values in all cols to a numpy version of cxg datatypes, - # then store the contents in the tiledb array A - A[:] = value - A.meta["cxg_schema"] = json.dumps(schema_hints) - - tiledb.consolidate(A_name, ctx=ctx) - - -def create_emb(e_name, emb): - """ - Embeddings are typically accessed with very large slices (or all of the embedding), - and do not benefit from overly aggressive compression due to their format. Given - this, we use: - * large tile size (1000) - * default compression level - """ - filters = tiledb.FilterList([tiledb.ZstdFilter()]) - attrs = [tiledb.Attr(dtype=emb.dtype, filters=filters)] - dims = [] - for d in range(emb.ndim): - shape = emb.shape - dims.append(tiledb.Dim(domain=(0, shape[d] - 1), tile=min(shape[d], 1000), dtype=np.uint32)) - domain = tiledb.Domain(*dims) - schema = tiledb.ArraySchema( - domain=domain, sparse=False, attrs=attrs, capacity=1_000_000, cell_order="row-major", tile_order="row-major" - ) - tiledb.DenseArray.create(e_name, schema) - - -def is_valid_embedding(adata, name, arr): - """ return True if this layout data is a valid array for front-end presentation: - * ndarray, with shape (n_obs, >= 2), dtype float/int/uint - * follows ScanPy embedding naming conventions - * with all values finite or NaN (no +Inf or -Inf) - """ - is_valid = type(name) == str and name.startswith("X_") and len(name) > 2 - is_valid = is_valid and type(arr) == np.ndarray and arr.dtype.kind in "fiu" - is_valid = is_valid and arr.shape[0] == adata.n_obs and arr.shape[1] >= 2 - is_valid = is_valid and not np.any(np.isinf(arr)) and not np.all(np.isnan(arr)) - return is_valid - - -def save_embeddings(container, adata, ctx): - for (name, value) in adata.obsm.items(): - if is_valid_embedding(adata, name, value): - e_name = f"{container}/{name[2:]}" - create_emb(e_name, value) - with tiledb.DenseArray(e_name, mode="w", ctx=ctx) as A: - A[:] = value - tiledb.consolidate(e_name, ctx=ctx) - log(1, f"\t\t...{name} embedding created") - - -def create_X(X_name, shape, is_sparse): - """ - The X matrix is accessed in both row and column oriented patterns, depending on the - particular operation. Because of the data type, default compression works best. - The tile size, (50, 100) for dense, and (512,2048) for sparse, - and global layout (row/col) was chosen empirically, by benchmarking - the current cellxgene backend. - """ - filters = tiledb.FilterList([tiledb.ZstdFilter()]) - attrs = [tiledb.Attr(dtype=np.float32, filters=filters)] - if is_sparse: - domain = tiledb.Domain( - tiledb.Dim(name="obs", domain=(0, shape[0] - 1), tile=min(shape[0], 512), dtype=np.uint32), - tiledb.Dim(name="var", domain=(0, shape[1] - 1), tile=min(shape[1], 2048), dtype=np.uint32), - ) - else: - domain = tiledb.Domain( - tiledb.Dim(name="obs", domain=(0, shape[0] - 1), tile=min(shape[0], 50), dtype=np.uint32), - tiledb.Dim(name="var", domain=(0, shape[1] - 1), tile=min(shape[1], 100), dtype=np.uint32), - ) - schema = tiledb.ArraySchema( - domain=domain, sparse=is_sparse, attrs=attrs, cell_order="row-major", tile_order="col-major" - ) - if is_sparse: - tiledb.SparseArray.create(X_name, schema) - else: - tiledb.DenseArray.create(X_name, schema) - - -def evaluate_for_sparse_encoding(xdata, sparse_threshold): - """ - This function determines if the X matrix has a sparsity below the sparse_threshold. - This function also returns the number of non-zeros encountered and number - of elements evaluated. This function may return before evaluating the whole X matrix - if it can be determined that X is not sparse enough. - """ - shape = xdata.shape - stride = min(int(np.power(10, np.around(np.log10(1e9 / shape[1])))), 10_000) - nnz = 0 - maxnnz = int(shape[0] * shape[1] * sparse_threshold / 100) - for row in range(0, shape[0], stride): - lim = min(row + stride, shape[0]) - a = xdata[row:lim, :] - if type(a) is not np.ndarray: - a = a.toarray() - nnz += np.count_nonzero(a) - if nnz > maxnnz: - return (False, nnz, lim * shape[1]) - log(2, "\t...rows", lim, "of", shape[0], "nnz", nnz, "nnz percent %5.2f%%" % (100 * nnz / (lim * shape[1]))) - - is_sparse = (100.0 * nnz / (shape[0] * shape[1])) < sparse_threshold - return (is_sparse, nnz, shape[0] * shape[1]) - - -def evaluate_for_sparse_column_shift_encoding(xdata, sparse_threshold): - """Column shift encoding works by taking the most common value in each column, then - subtracting that value from each element of the column. If each column mostly contains - its most common value, then the resulting matrix can be very sparse. - - This function determines if column shift encoding can be used to transform - the X matrix into a sparse matrix with a sparsity below the sparse_threshold. - If so, return the col_shift array that stores this encoding. - This function also returns the number of non-zeros encountered and number - of elements evaluated. This function may return before evaluating the whole X matrix - if it can be determined that X cannot benefit from column shift encoding. - """ - shape = xdata.shape - stride = max(1, 128_000_000 // shape[0]) - col_shift = np.zeros(shape[1]) - nnz = 0 - maxnnz = int(shape[0] * shape[1] * sparse_threshold / 100) - for col in range(0, shape[1], stride): - lim = min(col + stride, shape[1]) - a = xdata[:, col:lim] - if type(a) is not np.ndarray: - a = a.toarray() - m = mode(a) - col_shift[col:lim] = m.mode - nnz += shape[0] * (lim - col) - np.sum(m.count) - if nnz > maxnnz: - return (None, nnz, shape[0] * lim) - log(2, "\t...cols", lim, "of", shape[1], "nnz", nnz, "nnz percent %5.2f%%" % (100 * nnz / (lim * shape[0]))) - - is_sparse = (100.0 * nnz / (shape[0] * shape[1])) < sparse_threshold - return (col_shift if is_sparse else None, nnz, shape[0] * shape[1]) - - -def save_X(container, xdata, ctx, sparse_threshold, expect_sparse=False): - # Save X count matrix - X_name = f"{container}/X" - - shape = xdata.shape - log(1, "\t...shape:", str(shape)) - - col_shift = None - if sparse_threshold == 100: - is_sparse = True - elif sparse_threshold == 0: - is_sparse = False - else: - is_sparse, nnz, nelem = evaluate_for_sparse_encoding(xdata, sparse_threshold) - percent = 100.0 * nnz / nelem - if nelem != shape[0] * shape[1]: - log(1, "\t...sparse=", is_sparse, "non-zeros percent (estimate): %6.2f" % percent) - else: - log(1, "\t...sparse=", is_sparse, "non-zeros:", nnz, "percent: %6.2f" % percent) - - is_sparse = percent < sparse_threshold - if not is_sparse: - col_shift, nnz, nelem = evaluate_for_sparse_column_shift_encoding(xdata, sparse_threshold) - is_sparse = col_shift is not None - percent = 100.0 * nnz / nelem - if nelem != shape[0] * shape[1]: - log(1, "\t...sparse=", is_sparse, "col shift non-zeros percent (estimate): %6.2f" % percent) - else: - log(1, "\t...sparse=", is_sparse, "col shift non-zeros:", nnz, "percent: %6.2f" % percent) - - if expect_sparse is True and is_sparse is False: - return False - - create_X(X_name, shape, is_sparse) - stride = min(int(np.power(10, np.around(np.log10(1e9 / shape[1])))), 10_000) - if is_sparse: - if col_shift is not None: - log(1, "\t...output X as sparse matrix with column shift encoding") - X_col_shift_name = f"{container}/X_col_shift" - filters = tiledb.FilterList([tiledb.ZstdFilter()]) - attrs = [tiledb.Attr(dtype=np.float32, filters=filters)] - domain = tiledb.Domain(tiledb.Dim(domain=(0, shape[1] - 1), tile=min(shape[1], 5000), dtype=np.uint32)) - schema = tiledb.ArraySchema(domain=domain, attrs=attrs) - tiledb.DenseArray.create(X_col_shift_name, schema) - with tiledb.DenseArray(X_col_shift_name, mode="w", ctx=ctx) as X_col_shift: - X_col_shift[:] = col_shift - tiledb.consolidate(X_col_shift_name, ctx=ctx) - else: - log(1, "\t...output X as sparse matrix") - - with tiledb.SparseArray(X_name, mode="w", ctx=ctx) as X: - nnz = 0 - for row in range(0, shape[0], stride): - lim = min(row + stride, shape[0]) - a = xdata[row:lim, :] - if type(a) is not np.ndarray: - a = a.toarray() - if col_shift is not None: - a = a - col_shift - indices = np.nonzero(a) - trow = indices[0] + row - nnz += indices[0].shape[0] - X[trow, indices[1]] = a[indices[0], indices[1]] - log(2, "\t...rows", lim, "of", shape[0], "nnz", nnz, "sparse", nnz / (lim * shape[1])) - - else: - log(1, "\t...output X as dense matrix") - with tiledb.DenseArray(X_name, mode="w", ctx=ctx) as X: - for row in range(0, shape[0], stride): - lim = min(row + stride, shape[0]) - a = xdata[row:lim, :] - if type(a) is not np.ndarray: - a = a.toarray() - X[row:lim, :] = a - log(2, "\t...rows", row, "to", lim) - - tiledb.consolidate(X_name, ctx=ctx) - if hasattr(tiledb, "vacuum"): - tiledb.vacuum(X_name) - - return is_sparse - - -def save_metadata(container, metadata_dict): - """ - Save all dataset-wide metadata. This includes: - * CXG version - * dataset metadata, such as title and about link. - - Longer term, tiledb will have support for metadata on groups. Until - such feature exists, create an empty array and annotate that array. - - https://github.com/TileDB-Inc/TileDB-Py/issues/254 - """ - a_name = f"{container}/cxg_group_metadata" - with tiledb.from_numpy(a_name, np.zeros((1,))) as A: - pass - with tiledb.DenseArray(a_name, mode="w") as A: - for k, v in metadata_dict.items(): - A.meta[k] = v - - -def load_corpora_props(args, adata): - versions = corpora_get_versions_from_anndata(adata) - if versions is None: - return None - - [corpora_schema_version, corpora_encoding_version] = versions - corpora_props = corpora_get_props_from_anndata(adata) - version_is_supported = corpora_is_version_supported(corpora_schema_version, corpora_encoding_version) - if not version_is_supported or not corpora_props: - log(0, "ERROR: Unknown source file schema version is unsupported") - raise ValueError("Unsupported Corpora schema version") - - log(1, "FYI, file appears to be encoded using Corpora schema standards...") - if args.title is not None or args.about is not None: - log(0, "Warning: explicit specification of --title or --about will override Corpora schema fields.") - - return corpora_props - - -def create_cxg_group_metadata(adata, basefname, title=None, about=None, corpora_props=None, extract_colors=True): - - if corpora_props is not None: - # clobber encoding version to be OUR version, not the source H5AD encoding - corpora_props["version"].update({"corpora_encoding_version": CXG_VERSION}) - corpora_project_links = corpora_props.get("project_links", []) - corpora_about_link = next( - (link for link in corpora_project_links if (link.get("link_type", None) == "SUMMARY")), {} - ) - else: - corpora_about_link = {} - - title = title or corpora_about_link.get("link_name", basefname) - about = about or corpora_about_link.get("link_url") - - cxg_group_metadata = {"cxg_version": CXG_VERSION, "cxg_properties": json.dumps({"title": title, "about": about})} - if corpora_props is not None: - cxg_group_metadata.update({"corpora": json.dumps(corpora_props)}) - - if extract_colors: - try: - cxg_group_metadata["cxg_category_colors"] = json.dumps( - convert_anndata_category_colors_to_cxg_category_colors(adata) - ) - except ColorFormatException: - log( - 0, - "Warning: failed to extract colors from h5ad file! " - "Fix the h5ad file or rerun with --disable-custom-colors. See help for details.", - ) - - return cxg_group_metadata - - -def sanitize_keys(keys, update_keys=True): - """ - We need names to be safe to use as attribute names in tiledb. See: - TileDB-Inc/TileDB#1575 - TileDB-Inc/TileDB-Py#294 - This can be entirely removed once they add proper escaping. - - Args: list of keys - Returns: dict of {old_key: new_key, ...} - - Returned new keys will be both safe and unique. - - Masking out [~/.] and anything outside the ASCII range. - """ - p = re.compile(r"[^ -\.0-\[\]-\}]") - clean_keys = {k: p.sub("_", k) for k in keys} - - used_keys = set() - clean_unique_keys = {} - for k, v in clean_keys.items(): - if v not in used_keys: - used_keys.add(v) - clean_unique_keys[k] = v - continue - - # else, needs deduping. - counter = 1 - while True: - candidate_name = v + "-" + str(counter) - if candidate_name not in used_keys: - used_keys.add(candidate_name) - clean_unique_keys[k] = candidate_name - break - counter += 1 - - for k, v, in clean_unique_keys.items(): - if k != v: - if update_keys is False: - raise AnnotationCategoryNameError(f"{k} not a valid category name, please resubmit") - log(1, f"Renaming {k} to {v}") - return clean_unique_keys - - -def sanitize_df(df): - df.rename(columns=sanitize_keys(df.keys().tolist()), inplace=True) - - -def sanitize_mapping(mapping): - clean_keys = sanitize_keys([k for k in mapping.keys()]) - for old_key, new_key in clean_keys.items(): - if old_key != new_key: - mapping[new_key] = mapping[old_key] - del mapping[old_key] - - -def clean_all_column_names(adata): - sanitize_df(adata.obs) - sanitize_df(adata.var) - sanitize_mapping(adata.obsm) - - -if __name__ == "__main__": - main() diff --git a/server/converters/h5ad_data_file.py b/server/converters/h5ad_data_file.py new file mode 100644 index 00000000..c8223822 --- /dev/null +++ b/server/converters/h5ad_data_file.py @@ -0,0 +1,250 @@ +import json +import logging +from os import path + +import anndata +import numpy as np +import tiledb + +from server.common.colors import convert_anndata_category_colors_to_cxg_category_colors +from server.common.corpora import corpora_get_props_from_anndata +from server.common.errors import ColorFormatException +from server.common.utils.cxg_constants import CxgConstants +from server.common.utils.cxg_generation_utils import ( + convert_dictionary_to_cxg_group, + convert_dataframe_to_cxg_array, + convert_ndarray_to_cxg_dense_array, + convert_matrix_to_cxg_array, +) +from server.common.utils.matrix_utils import is_matrix_sparse, get_column_shift_encode_for_matrix + + +class H5ADDataFile: + """ Class encapsulating required information about an H5AD datafile that ultimately will be transformed into + another format (currently just CXG is supported). """ + + def __init__( + self, + input_filename, + backed=False, + dataset_title=None, + dataset_about=None, + obs_index_column_name=None, + vars_index_column_name=None, + use_corpora_schema=True, + ): + self.input_filename = input_filename + self.backed = backed + self.dataset_title = dataset_title + self.dataset_about = dataset_about + self.obs_index_column_name = obs_index_column_name + self.vars_index_column_name = vars_index_column_name + + self.use_corpora_schema = use_corpora_schema + + self.validate_input_file_type() + + self.extract_anndata_elements_from_file() + self.extract_metadata_about_dataset() + + self.validate_anndata() + + def to_cxg(self, output_cxg_directory, sparse_threshold, convert_anndata_colors_to_cxg_colors=True): + """ + Writes the following attributes of the anndata to CXG: 1) the metadata as metadata attached to an empty + DenseArray, 2) the obs DataFrame as a DenseArray, 3) the var DataFrame as a DenseArray, 4) all valid + embeddings stored in obsm, each one as a DenseArray, 5) the main X matrix of the anndata as either a + SparseArray or DenseArray based on the `sparse_threshold`, and optionally 6) the column shift of the main X + matrix that might turn an otherwise Dense matrix into a Sparse matrix. + """ + + logging.info("Beginning writing to CXG.") + ctx = tiledb.Ctx( + { + "sm.num_reader_threads": 32, + "sm.num_writer_threads": 32, + "sm.consolidation.buffer_size": 1 * 1024 * 1024 * 1024, + } + ) + + tiledb.group_create(output_cxg_directory, ctx=ctx) + logging.info(f"\t...group created, with name {output_cxg_directory}") + + convert_dictionary_to_cxg_group( + output_cxg_directory, self.generate_cxg_metadata(convert_anndata_colors_to_cxg_colors) + ) + logging.info("\t...dataset metadata saved") + + convert_dataframe_to_cxg_array(output_cxg_directory, "obs", self.obs, self.obs_index_column_name, ctx) + logging.info("\t...dataset obs dataframe saved") + + convert_dataframe_to_cxg_array(output_cxg_directory, "var", self.var, self.var_index_column_name, ctx) + logging.info("\t...dataset var dataframe saved") + + self.write_anndata_embeddings_to_cxg(output_cxg_directory, ctx) + logging.info("\t...dataset embeddings saved") + + self.write_anndata_x_matrix_to_cxg(output_cxg_directory, ctx, sparse_threshold) + logging.info("\t...dataset X matrix saved") + + logging.info("Completed writing to CXG.") + + def write_anndata_x_matrix_to_cxg(self, output_cxg_directory, ctx, sparse_threshold): + matrix_container = f"{output_cxg_directory}/X" + + x_matrix_data = self.anndata.X + is_sparse = is_matrix_sparse(x_matrix_data, sparse_threshold) + if not is_sparse: + col_shift = get_column_shift_encode_for_matrix(x_matrix_data, sparse_threshold) + is_sparse = col_shift is not None + else: + col_shift = None + + if col_shift is not None: + logging.info("Converting matrix X as sparse matrix with column shift encoding") + x_col_shift_name = f"{output_cxg_directory}/X_col_shift" + convert_ndarray_to_cxg_dense_array(x_col_shift_name, col_shift, ctx) + + convert_matrix_to_cxg_array(matrix_container, x_matrix_data, is_sparse, ctx, col_shift) + + tiledb.consolidate(matrix_container, ctx=ctx) + if hasattr(tiledb, "vacuum"): + tiledb.vacuum(matrix_container) + + def write_anndata_embeddings_to_cxg(self, output_cxg_directory, ctx): + def is_valid_embedding(adata, embedding_name, embedding_array): + """ + Returns true if this layout data is a valid array for front-end presentation with the following criteria: + * ndarray, with shape (n_obs, >= 2), dtype float/int/uint + * follows ScanPy embedding naming conventions + * with all values finite or NaN (no +Inf or -Inf) + """ + + is_valid = isinstance(embedding_name, str) and embedding_name.startswith("X_") and len(embedding_name) > 2 + is_valid = is_valid and isinstance(embedding_array, np.ndarray) and embedding_array.dtype.kind in "fiu" + is_valid = is_valid and embedding_array.shape[0] == adata.n_obs and embedding_array.shape[1] >= 2 + is_valid = is_valid and not np.any(np.isinf(embedding_array)) and not np.all(np.isnan(embedding_array)) + return is_valid + + embedding_container = f"{output_cxg_directory}/emb" + tiledb.group_create(embedding_container, ctx=ctx) + + for embedding_name, embedding_values in self.anndata.obsm.items(): + if is_valid_embedding(self.anndata, embedding_name, embedding_values): + embedding_name = f"{embedding_container}/{embedding_name[2:]}" + convert_ndarray_to_cxg_dense_array(embedding_name, embedding_values, ctx) + logging.info(f"\t\t...{embedding_name} embedding created") + + def generate_cxg_metadata(self, convert_anndata_colors_to_cxg_colors): + """ + Return a dictionary containing metadata about CXG dataset. This include data about the version as well as + Corpora schema properties if they exist, among other pieces of metadata. + """ + + cxg_group_metadata = { + "cxg_version": CxgConstants.CXG_VERSION, + "cxg_properties": json.dumps({"title": self.dataset_title, "about": self.dataset_about}), + } + if self.corpora_properties is not None: + cxg_group_metadata["corpora"] = json.dumps(self.corpora_properties) + + if convert_anndata_colors_to_cxg_colors: + try: + cxg_group_metadata["cxg_category_colors"] = json.dumps( + convert_anndata_category_colors_to_cxg_category_colors(self.anndata) + ) + except ColorFormatException: + logging.warning( + "Failed to extract colors from H5AD file! Fix the H5AD file or rerun with " + "--disable-custom-colors. See help for more details." + ) + + return cxg_group_metadata + + def validate_input_file_type(self): + """ + Validate that the input file is of a type that we can handle. Currently the only valid file type is `.h5ad`. + """ + + if not self.input_filename.endswith(".h5ad"): + raise Exception(f"Cannot process input file {self.input_filename}. File must be an H5AD.") + + if self.dataset_title or self.dataset_about: + logging.warning( + "If you convert this dataset into CXG and you explicit specify values for the dataset title metadata " + "or the dataset about metadata, it will override any metadata that is extracted as part of the " + "Corpora schema fields." + ) + + def validate_anndata(self): + if not self.var.index.is_unique: + raise ValueError("Variable index in AnnData object is not unique.") + if not self.obs.index.is_unique: + raise ValueError("Observation index in AnnData object is not unique.") + + def extract_anndata_elements_from_file(self): + logging.info(f"Reading in AnnData dataset: {path.basename(self.input_filename)}") + self.anndata = anndata.read_h5ad(self.input_filename, backed="r" if self.backed else None) + logging.info("Completed reading in AnnData dataset!") + + self.obs = self.transform_dataframe_index_into_column(self.anndata.obs, "obs", self.obs_index_column_name) + self.var = self.transform_dataframe_index_into_column(self.anndata.var, "var", self.vars_index_column_name) + + def extract_metadata_about_dataset(self): + """ + Extract metadata information about the dataset that upon conversion will be saved as group metadata with the + CXG that is generated. This metadata information includes Corpora schema properties, the dataset title and + a link that details more information about the dataset. + """ + + self.corpora_properties = corpora_get_props_from_anndata(self.anndata) if self.use_corpora_schema else None + if self.corpora_properties is None and self.use_corpora_schema: + # If the return value is None, this means that we were not able to figure out what version of the Corpora + # schema the object is using and therefore cannot extract any properties. + raise ValueError("Unknown source file schema version is unsupported.") + + # The title and about properties of the dataset are set by the following order: if they are explicitly defined + # then use the explicit value. If the dataset is a Corpora-schema based schema, then extract the title and about + # from the corpora_properties. Otherwise, use the input filename (only for title, about will be blank). + if self.corpora_properties: + corpora_project_links = self.corpora_properties.get("project_links", []) + corpora_about_link = next( + (link for link in corpora_project_links if (link.get("link_type", None) == "SUMMARY")), {} + ) + else: + corpora_about_link = {} + + filename = path.splitext(path.basename(self.input_filename))[0] + + self.dataset_title = self.dataset_title if self.dataset_title else corpora_about_link.get("link_name", filename) + self.dataset_about = self.dataset_about if self.dataset_about else corpora_about_link.get("link_url") + + def transform_dataframe_index_into_column(self, dataframe, dataframe_name, index_column_name): + """ + Convert the dataframe's index into another column in the dataframe. If an index_column_name is specified, + use that column as the index instead. + """ + + if index_column_name is None: + # Create a unique column name for the index. + suffix = 0 + while f"name_{suffix}" in dataframe.columns: + suffix += 1 + index_column_name = f"name_{suffix}" + + # Turn the index into a normal column + dataframe.rename_axis(index_column_name, inplace=True) + dataframe.reset_index(inplace=True) + + elif index_column_name in dataframe.columns: + # User has specified alternative column for unique names, and it exists + if not dataframe[index_column_name].is_unique: + raise KeyError( + f"Values in {dataframe_name}.{index_column_name} must be unique. Please prepare data to contain " + f"unique values." + ) + else: + raise KeyError(f"Column {index_column_name} does not exist.") + + setattr(self, f"{dataframe_name}_index_column_name", index_column_name) + return dataframe diff --git a/server/test/unit/common/test_api.py b/server/test/unit/common/test_api.py index 3e2c79b9..ee979b1f 100644 --- a/server/test/unit/common/test_api.py +++ b/server/test/unit/common/test_api.py @@ -423,11 +423,7 @@ class EndPointsAnndataAnnotations(unittest.TestCase, EndPointsAnnotations): cls.data, cls.tmp_dir, cls.annotations = data_with_tmp_annotations( MatrixDataType.H5AD, annotations_fixture=True ) - cls._setupClass(cls, [ - "--annotations-file", - cls.annotations.output_file, - cls.data.get_location(), - ]) + cls._setupClass(cls, ["--annotations-file", cls.annotations.output_file, cls.data.get_location(), ]) @classmethod def tearDownClass(cls): diff --git a/server/test/unit/common/test_corpora.py b/server/test/unit/common/test_corpora.py index f502244c..1a0222dc 100644 --- a/server/test/unit/common/test_corpora.py +++ b/server/test/unit/common/test_corpora.py @@ -1,9 +1,10 @@ -import unittest -import anndata import json -import tempfile import shutil +import tempfile +import unittest from http import HTTPStatus + +import anndata import requests from server.common.corpora import ( @@ -104,7 +105,7 @@ class CorporaRESTAPITest(unittest.TestCase): "project_links": json.dumps([ {"link_name": "test link", "link_type": "SUMMARY", "link_url": "https://a.u.r.l/"} ]), - "default_embedding": "X_tsne" + "default_embedding": "X_tsne", } adata.uns.update(corpora_props) adata.write(path) diff --git a/server/test/unit/common/utils/test_cxg_generation_utils.py b/server/test/unit/common/utils/test_cxg_generation_utils.py new file mode 100644 index 00000000..57893913 --- /dev/null +++ b/server/test/unit/common/utils/test_cxg_generation_utils.py @@ -0,0 +1,149 @@ +import json +import unittest +from os import popen, path, mkdir +from shutil import rmtree +from uuid import uuid4 + +import numpy as np +import tiledb +from pandas import Series, DataFrame + +from server.common.utils.cxg_generation_utils import (convert_dictionary_to_cxg_group, convert_dataframe_to_cxg_array, + convert_ndarray_to_cxg_dense_array, convert_matrix_to_cxg_array) + +PROJECT_ROOT = popen("git rev-parse --show-toplevel").read().strip() + + +class TestCxgGenerationUtils(unittest.TestCase): + def setUp(self): + self.testing_cxg_temp_directory = f"{PROJECT_ROOT}/server/test/fixtures/{uuid4()}" + mkdir(self.testing_cxg_temp_directory) + + def tearDown(self): + if path.isdir(self.testing_cxg_temp_directory): + rmtree(self.testing_cxg_temp_directory) + + def test__convert_dictionary_to_cxg_group__writes_successfully(self): + random_dictionary = {"cookies": "chocolate_chip", "brownies": "chocolate", "cake": "double chocolate"} + dictionary_name = "favorite_desserts" + expected_array_directory = f"{self.testing_cxg_temp_directory}/{dictionary_name}" + + convert_dictionary_to_cxg_group(self.testing_cxg_temp_directory, random_dictionary, + group_metadata_name=dictionary_name) + + array = tiledb.open(expected_array_directory) + actual_stored_metadata = dict(array.meta.items()) + + self.assertTrue(path.isdir(expected_array_directory)) + self.assertTrue(isinstance(array, tiledb.DenseArray)) + self.assertEqual(random_dictionary, actual_stored_metadata) + + def test__convert_dataframe_to_cxg_array__writes_successfully(self): + random_int_category = Series(data=[3, 1, 2, 4], dtype=np.int64) + random_bool_category = Series(data=[True, True, False, True], dtype=np.bool_) + random_dataframe_name = f"random_dataframe_{uuid4()}" + random_dataframe = DataFrame(data={"int_category": random_int_category, "bool_category": random_bool_category}) + + convert_dataframe_to_cxg_array(self.testing_cxg_temp_directory, random_dataframe_name, random_dataframe, + "int_category", tiledb.Ctx()) + + expected_array_directory = f"{self.testing_cxg_temp_directory}/{random_dataframe_name}" + expected_array_metadata = { + "cxg_schema": json.dumps({"int_category": {"type": "int32"}, "bool_category": {"type": "boolean"}, + "index": "int_category"})} + + actual_stored_dataframe_array = tiledb.open(expected_array_directory) + actual_stored_dataframe_metadata = dict(actual_stored_dataframe_array.meta.items()) + + self.assertTrue(path.isdir(expected_array_directory)) + self.assertTrue(isinstance(actual_stored_dataframe_array, tiledb.DenseArray)) + self.assertDictEqual(expected_array_metadata, actual_stored_dataframe_metadata) + self.assertTrue((actual_stored_dataframe_array[0:4]["int_category"] == random_int_category.to_numpy()).all()) + self.assertTrue((actual_stored_dataframe_array[0:4]["bool_category"] == random_bool_category.to_numpy()).all()) + + def test__convert_ndarray_to_cxg_dense_array__writes_successfully(self): + ndarray = np.random.rand(3, 2) + ndarray_name = f"{self.testing_cxg_temp_directory}/awesome_ndarray_{uuid4()}" + + convert_ndarray_to_cxg_dense_array(ndarray_name, ndarray, tiledb.Ctx()) + + actual_stored_array = tiledb.open(ndarray_name) + + self.assertTrue(path.isdir(ndarray_name)) + self.assertTrue(isinstance(actual_stored_array, tiledb.DenseArray)) + self.assertTrue((actual_stored_array[:, :] == ndarray).all()) + + def test__convert_matrix_to_cxg_array__dense_array_writes_successfully(self): + matrix = np.float32(np.random.rand(3, 2)) + matrix_name = f"{self.testing_cxg_temp_directory}/awesome_matrix_{uuid4()}" + + convert_matrix_to_cxg_array(matrix_name, matrix, False, tiledb.Ctx()) + + actual_stored_array = tiledb.open(matrix_name) + + self.assertTrue(path.isdir(matrix_name)) + self.assertTrue(isinstance(actual_stored_array, tiledb.DenseArray)) + self.assertTrue((actual_stored_array[:, :] == matrix).all()) + + def test__convert_matrix_to_cxg_array__sparse_array_only_store_nonzeros_empty_array(self): + matrix = np.zeros([3, 2]) + matrix_name = f"{self.testing_cxg_temp_directory}/awesome_zero_matrix_{uuid4()}" + + convert_matrix_to_cxg_array(matrix_name, matrix, True, tiledb.Ctx()) + + actual_stored_array = tiledb.open(matrix_name) + + self.assertTrue(path.isdir(matrix_name)) + self.assertTrue(isinstance(actual_stored_array, tiledb.SparseArray)) + self.assertTrue(actual_stored_array[:, :][''].size == 0) + + def test__convert_matrix_to_cxg_array__sparse_array_only_store_nonzeros(self): + matrix = np.zeros([3, 3]) + matrix[0, 0] = 1 + matrix[1, 1] = 1 + matrix[2, 2] = 2 + matrix_name = f"{self.testing_cxg_temp_directory}/awesome_sparse_matrix_{uuid4()}" + + convert_matrix_to_cxg_array(matrix_name, matrix, True, tiledb.Ctx()) + + actual_stored_array = tiledb.open(matrix_name) + + self.assertTrue(path.isdir(matrix_name)) + self.assertTrue(isinstance(actual_stored_array, tiledb.SparseArray)) + self.assertTrue(actual_stored_array[0, 0][''] == 1) + self.assertTrue(actual_stored_array[1, 1][''] == 1) + self.assertTrue(actual_stored_array[2, 2][''] == 2) + self.assertTrue(actual_stored_array[:, :][''].size == 3) + + def test__convert_matrix_to_cxg_array__sparse_array_with_column_encoding_empty_array(self): + matrix_name = f"{self.testing_cxg_temp_directory}/awesome_column_shift_matrix_{uuid4()}" + matrix = np.ones((3, 2)) + # The column shift will be equal to the matrix since subtracting the column shift from the matrix will create + # a matrix of zeros which is sparse. + column_shift = np.ones((3, 2)) + + convert_matrix_to_cxg_array(matrix_name, matrix, True, tiledb.Ctx(), + column_shift_for_sparse_encoding=column_shift) + + actual_stored_array = tiledb.open(matrix_name) + + self.assertTrue(path.isdir(matrix_name)) + self.assertTrue(isinstance(actual_stored_array, tiledb.SparseArray)) + self.assertTrue(actual_stored_array[:, :][''].size == 0) + + def test__convert_matrix_to_cxg_array__sparse_array_with_column_encoding_partial_array(self): + matrix_name = f"{self.testing_cxg_temp_directory}/awesome_column_shift_matrix_{uuid4()}" + matrix = np.ones((2, 2)) + # Only column shift the first column of ones. + column_shift = np.array([[1, 0], [1, 0]]) + + convert_matrix_to_cxg_array(matrix_name, matrix, True, tiledb.Ctx(), + column_shift_for_sparse_encoding=column_shift) + + actual_stored_array = tiledb.open(matrix_name) + + self.assertTrue(path.isdir(matrix_name)) + self.assertTrue(isinstance(actual_stored_array, tiledb.SparseArray)) + self.assertTrue(actual_stored_array[0, 1][''] == 1) + self.assertTrue(actual_stored_array[1, 1][''] == 1) + self.assertTrue(actual_stored_array[:, :][''].size == 2) diff --git a/server/test/unit/common/utils/test_type_conversion_utils.py b/server/test/unit/common/utils/test_type_conversion_utils.py index a5bb0395..9778bfca 100644 --- a/server/test/unit/common/utils/test_type_conversion_utils.py +++ b/server/test/unit/common/utils/test_type_conversion_utils.py @@ -2,10 +2,10 @@ import unittest from unittest.mock import patch import numpy as np -from pandas import Series +from pandas import Series, DataFrame from server.common.utils.type_conversion_utils import can_cast_to_float32, can_cast_to_int32, get_dtype_of_array, \ - get_schema_type_hint_of_array + get_schema_type_hint_of_array, get_dtypes_and_schemas_of_dataframe class TestTypeConversionUtils(unittest.TestCase): @@ -119,3 +119,17 @@ class TestTypeConversionUtils(unittest.TestCase): i=test_type_index): array = Series(data=[], dtype=types[test_type_index]) self.assertEqual(get_schema_type_hint_of_array(array), expected_schema_hints[test_type_index]) + + def test__get_dtypes_and_schemas_of_dataframe__dtype_and_schema_returns_as_expected(self): + float_array = Series(data=[1, 2, 3], dtype=np.dtype(np.float64)) + category_array = Series(data=["a", "b", "b"], dtype="category") + dataframe = DataFrame({"float_array": float_array, "category_array": category_array}) + + expected_data_types_dict = {"float_array": np.float32, "category_array": np.unicode} + expected_schema_type_hints_dict = {"float_array": {"type": "float32"}, + "category_array": {"type": "categorical", "categories": ["a", "b"]}} + + actual_dataframe_data_types, actual_dataframe_schema_type_hints = get_dtypes_and_schemas_of_dataframe(dataframe) + + self.assertEqual(expected_data_types_dict, actual_dataframe_data_types) + self.assertEqual(expected_schema_type_hints_dict, actual_dataframe_schema_type_hints) diff --git a/server/test/unit/compute/test_diffexp_cxg.py b/server/test/unit/compute/test_diffexp_cxg.py index 7a6aabdd..30dc7472 100644 --- a/server/test/unit/compute/test_diffexp_cxg.py +++ b/server/test/unit/compute/test_diffexp_cxg.py @@ -1,14 +1,16 @@ +import os +import tempfile import unittest -from server.data_common.matrix_loader import MatrixDataLoader -from server.test import PROJECT_ROOT, app_config, FIXTURES_ROOT + +import numpy as np + import server.compute.diffexp_cxg as diffexp_cxg import server.compute.diffexp_generic as diffexp_generic -from server.converters.cxgtool import write_cxg, create_cxg_group_metadata -from server.test.performance.create_test_matrix import create_test_h5ad +from server.converters.h5ad_data_file import H5ADDataFile from server.data_common.fbs.matrix import encode_matrix_fbs, decode_matrix_fbs -import numpy as np -import tempfile -import os +from server.data_common.matrix_loader import MatrixDataLoader +from server.test import PROJECT_ROOT, app_config, FIXTURES_ROOT +from server.test.performance.create_test_matrix import create_test_h5ad class DiffExpTest(unittest.TestCase): @@ -98,21 +100,22 @@ class DiffExpTest(unittest.TestCase): def sparse_diffexp(self, apply_col_shift): with tempfile.TemporaryDirectory() as dirname: # create a sparse matrix - h5adfile = os.path.join(dirname, "sparse.h5ad") - create_test_h5ad(h5adfile, 2000, 2000, 10, apply_col_shift) - adaptor_anndata = self.load_dataset(h5adfile, extra_dataset_config=dict(embeddings__names=[])) - adata = adaptor_anndata.data + h5adfile_path = os.path.join(dirname, "sparse.h5ad") + create_test_h5ad(h5adfile_path, 2000, 2000, 10, apply_col_shift) + + h5ad_file_to_convert = H5ADDataFile(h5adfile_path, use_corpora_schema=False) sparsename = os.path.join(dirname, "sparse.cxg") - cxg_group_metadata = create_cxg_group_metadata(adata=adata, basefname="sparse.h5ad", title="sparse",) - write_cxg(adata=adata, container=sparsename, cxg_group_metadata=cxg_group_metadata, sparse_threshold=11) + h5ad_file_to_convert.to_cxg(sparsename, 11, True) + + adaptor_anndata = self.load_dataset(h5adfile_path, extra_dataset_config=dict(embeddings__names=[])) + adaptor_sparse = self.load_dataset(sparsename) assert adaptor_sparse.open_array("X").schema.sparse assert adaptor_sparse.has_array("X_col_shift") == apply_col_shift densename = os.path.join(dirname, "dense.cxg") - cxg_group_metadata = create_cxg_group_metadata(adata=adata, basefname="dense.h5ad", title="dense",) - write_cxg(adata=adata, container=densename, cxg_group_metadata=cxg_group_metadata, sparse_threshold=0) + h5ad_file_to_convert.to_cxg(densename, True, 0) adaptor_dense = self.load_dataset(densename) assert not adaptor_dense.open_array("X").schema.sparse assert not adaptor_dense.has_array("X_col_shift") diff --git a/server/test/unit/converters/test_cxgtool.py b/server/test/unit/converters/test_cxgtool.py deleted file mode 100644 index 46e26b48..00000000 --- a/server/test/unit/converters/test_cxgtool.py +++ /dev/null @@ -1,41 +0,0 @@ -import shutil -import unittest - -import anndata - -from server.common.data_locator import DataLocator -from server.converters.cxgtool import write_cxg, create_cxg_group_metadata -from server.data_cxg.cxg_adaptor import CxgAdaptor -from server.test import PROJECT_ROOT, app_config, random_string -from server.test.fixtures.fixtures import pbmc3k_colors - - -class TestCxgAdaptor(unittest.TestCase): - def setUp(self) -> None: - self.fixtures = [] - - def tearDown(self) -> None: - try: - for data_locator in self.fixtures: - print("REMOVING ", data_locator) - shutil.rmtree(data_locator) - except FileNotFoundError: - pass - - def test_cxg_category_colors(self): - data = self.convert_pbmc3k(extract_colors=True) - self.assertEqual(data.get_colors(), pbmc3k_colors) - data = self.convert_pbmc3k(extract_colors=False) - self.assertEqual(data.get_colors(), {}) - - def convert_pbmc3k(self, **kwargs): - rand_str = random_string(8) - data_locator = f"/tmp/test_{rand_str}.cxg" - self.fixtures.append(data_locator) - source_h5ad = anndata.read_h5ad(f"{PROJECT_ROOT}/example-dataset/pbmc3k.h5ad") - cxg_group_metadata = create_cxg_group_metadata( - adata=source_h5ad, basefname="pbmc3k.h5ad", title="pbmc3k", **kwargs - ) - write_cxg(adata=source_h5ad, container=data_locator, cxg_group_metadata=cxg_group_metadata) - config = app_config(data_locator) - return CxgAdaptor(DataLocator(data_locator), config) diff --git a/server/test/unit/converters/test_h5ad_data_file.py b/server/test/unit/converters/test_h5ad_data_file.py new file mode 100644 index 00000000..f8587ebd --- /dev/null +++ b/server/test/unit/converters/test_h5ad_data_file.py @@ -0,0 +1,235 @@ +import json +import unittest +from glob import glob +from os import popen, remove, path +from shutil import rmtree +from uuid import uuid4 + +import anndata +import numpy as np +from pandas import Series, DataFrame + +from server.common.utils.corpora_constants import CorporaConstants +from server.converters.h5ad_data_file import H5ADDataFile + +PROJECT_ROOT = popen("git rev-parse --show-toplevel").read().strip() + + +class TestH5ADDataFile(unittest.TestCase): + + def setUp(self): + self.sample_anndata = self._create_sample_anndata_dataset() + self.sample_h5ad_filename = self._write_anndata_to_file(self.sample_anndata) + + self.sample_output_directory = path.splitext(self.sample_h5ad_filename)[0] + ".cxg" + + def tearDown(self): + if self.sample_h5ad_filename: + remove(self.sample_h5ad_filename) + + if path.isdir(self.sample_output_directory): + rmtree(self.sample_output_directory) + + def test__create_h5ad_data_file__non_h5ad_raises_exception(self): + non_h5ad_filename = "my_fancy_dataset.csv" + + with self.assertRaises(Exception) as exception_context: + H5ADDataFile(non_h5ad_filename) + + self.assertIn("File must be an H5AD", str(exception_context.exception)) + + def test__create_h5ad_data_file__assert_warning_outputted_if_dataset_title_or_about_given(self): + with self.assertLogs(level="WARN") as logger: + H5ADDataFile(self.sample_h5ad_filename, dataset_title="My Awesome Dataset", + dataset_about="http://www.awesomedataset.com", use_corpora_schema=False) + + self.assertIn("will override any metadata that is extracted", logger.output[0]) + + def test__create_h5ad_data_file__reads_anndata_successfully(self): + h5ad_file = H5ADDataFile(self.sample_h5ad_filename, use_corpora_schema=False) + + self.assertTrue((h5ad_file.anndata.X == self.sample_anndata.X).all()) + self.assertEqual(h5ad_file.anndata.obs.sort_index(inplace=True), + self.sample_anndata.obs.sort_index(inplace=True)) + self.assertEqual(h5ad_file.anndata.var.sort_index(inplace=True), + self.sample_anndata.var.sort_index(inplace=True)) + + for key in h5ad_file.anndata.obsm.keys(): + self.assertIn(key, self.sample_anndata.obsm.keys()) + self.assertTrue((h5ad_file.anndata.obsm[key] == self.sample_anndata.obsm[key]).all()) + + for key in self.sample_anndata.obsm.keys(): + self.assertIn(key, h5ad_file.anndata.obsm.keys()) + self.assertTrue((h5ad_file.anndata.obsm[key] == self.sample_anndata.obsm[key]).all()) + + def test__create_h5ad_data_file__copies_index_of_obs_and_var_to_column(self): + h5ad_file = H5ADDataFile(self.sample_h5ad_filename, use_corpora_schema=False) + + # The automatic name chosen for the index should be "name_0" + self.assertNotIn("name_0", self.sample_anndata.obs.columns) + self.assertIn("name_0", h5ad_file.obs.columns) + + self.assertNotIn("name_0", self.sample_anndata.var.columns) + self.assertIn("name_0", h5ad_file.var.columns) + + def test__create_h5ad_data_file__no_copy_if_obs_and_var_index_names_specified(self): + h5ad_file = H5ADDataFile(self.sample_h5ad_filename, use_corpora_schema=False, + obs_index_column_name="float_category", vars_index_column_name="int_category") + + self.assertNotIn("name_0", h5ad_file.obs.columns) + self.assertNotIn("name_0", h5ad_file.var.columns) + + def test__create_h5ad_data_file__obs_and_var_index_names_specified_not_unique_raises_exception(self): + + with self.assertRaises(Exception) as exception_context: + H5ADDataFile(self.sample_h5ad_filename, use_corpora_schema=False, + obs_index_column_name="float_category", vars_index_column_name="bool_category") + + self.assertIn("Please prepare data to contain unique values", str(exception_context.exception)) + + def test__create_h5ad_data_file__obs_and_var_index_names_specified_doesnt_exist_raises_exception(self): + with self.assertRaises(Exception) as exception_context: + H5ADDataFile(self.sample_h5ad_filename, use_corpora_schema=False, + obs_index_column_name="unknown_category", vars_index_column_name="i_dont_exist") + + self.assertIn("does not exist", str(exception_context.exception)) + + def test__create_h5ad_data_file__extract_about_and_title_from_dataset(self): + h5ad_file = H5ADDataFile(self.sample_h5ad_filename) + + self.assertEqual(h5ad_file.dataset_title, "random_link_name") + self.assertEqual(h5ad_file.dataset_about, "www.link.com") + + def test__create_h5ad_data_file__inputted_dataset_title_and_about_overrides_extracted(self): + h5ad_file = H5ADDataFile(self.sample_h5ad_filename, dataset_about="override_about", + dataset_title="override_title") + + self.assertEqual(h5ad_file.dataset_title, "override_title") + self.assertEqual(h5ad_file.dataset_about, "override_about") + + def test__to_cxg__simple_anndata_no_corpora_and_sparse(self): + h5ad_file = H5ADDataFile(self.sample_h5ad_filename, use_corpora_schema=False) + h5ad_file.to_cxg(self.sample_output_directory, 100) + + self._validate_expected_generated_list_of_tiledb_files() + + def test__to_cxg__simple_anndata_with_corpora_and_sparse(self): + h5ad_file = H5ADDataFile(self.sample_h5ad_filename) + h5ad_file.to_cxg(self.sample_output_directory, 100) + + self._validate_expected_generated_list_of_tiledb_files() + + def test__to_cxg__simple_anndata_no_corpora_and_dense(self): + h5ad_file = H5ADDataFile(self.sample_h5ad_filename, use_corpora_schema=False) + h5ad_file.to_cxg(self.sample_output_directory, 0) + + self._validate_expected_generated_list_of_tiledb_files() + + def test__to_cxg__simple_anndata_with_corpora_and_dense(self): + h5ad_file = H5ADDataFile(self.sample_h5ad_filename) + h5ad_file.to_cxg(self.sample_output_directory, 0) + + self._validate_expected_generated_list_of_tiledb_files() + + def test__to_cxg__with_sparse_column_encoding(self): + anndata = self._create_sample_anndata_dataset() + anndata.X = np.ones((3, 4)) + sparse_with_column_shift_filename = self._write_anndata_to_file(anndata) + + h5ad_file = H5ADDataFile(sparse_with_column_shift_filename) + h5ad_file.to_cxg(self.sample_output_directory, 50) + + self._validate_expected_generated_list_of_tiledb_files(has_column_encoding=True) + + # Clean up + remove(sparse_with_column_shift_filename) + + def _validate_expected_generated_list_of_tiledb_files(self, has_column_encoding=False): + expected_directories, expected_obs_files, expected_var_files = \ + self._get_expected_generated_list_of_tiledb_files() + + for directory in expected_directories: + self.assertTrue(path.isdir(directory)) + + for obs_file in expected_obs_files: + expected_location_of_obs_file = f"{self.sample_output_directory}/obs/*/{obs_file}" + self.assertTrue(path.isfile(glob(expected_location_of_obs_file)[0])) + + for var_file in expected_var_files: + expected_location_of_var_file = f"{self.sample_output_directory}/var/*/{var_file}" + self.assertTrue(path.isfile(glob(expected_location_of_var_file)[0])) + + if has_column_encoding: + self.assertTrue(path.isdir(f"{self.sample_output_directory}/X_col_shift")) + + def _get_expected_generated_list_of_tiledb_files(self): + + # Expected directories + metadata_directory = f"{self.sample_output_directory}/cxg_group_metadata" + main_x_directory = f"{self.sample_output_directory}/X" + overall_embedding_directory = f"{self.sample_output_directory}/emb" + specific_embedding_directory = f"{self.sample_output_directory}/emb/awesome_embedding" + obs_directory = f"{self.sample_output_directory}/obs" + var_directory = f"{self.sample_output_directory}/var" + + # Obs files + obs_files = [] + obs_files.append("name_0.tdb") + obs_files.append("name_0_var.tdb") + obs_files.append("string_category.tdb") + obs_files.append("string_category_var.tdb") + obs_files.append("float_category.tdb") + + # Var files + var_files = [] + var_files.append("name_0.tdb") + var_files.append("name_0_var.tdb") + var_files.append("bool_category.tdb") + var_files.append("int_category.tdb") + + return [metadata_directory, main_x_directory, overall_embedding_directory, specific_embedding_directory, + obs_directory, var_directory], obs_files, var_files + + def _write_anndata_to_file(self, anndata): + temporary_filename = f"{PROJECT_ROOT}/server/test/fixtures/{uuid4()}.h5ad" + anndata.write(temporary_filename) + + return temporary_filename + + def _create_sample_anndata_dataset(self): + # Create X + X = np.random.rand(3, 4) + + # Create obs + random_string_category = Series(data=["a", "b", "b"], dtype="category") + random_float_category = Series(data=[3.2, 1.1, 2.2], dtype=np.float32) + obs_dataframe = DataFrame( + data={"string_category": random_string_category, "float_category": random_float_category}) + obs = obs_dataframe + + # Create vars + random_int_category = Series(data=[3, 1, 2, 4], dtype=np.int32) + random_bool_category = Series(data=[True, True, False, True], dtype=np.bool_) + var_dataframe = DataFrame(data={"int_category": random_int_category, "bool_category": random_bool_category}) + var = var_dataframe + + # Create embeddings + random_embedding = np.random.rand(3, 2) + obsm = {"X_awesome_embedding": random_embedding} + + # Create uns corpora metadata + uns = {} + for metadata_field in CorporaConstants.REQUIRED_SIMPLE_METADATA_FIELDS: + uns[metadata_field] = "random" + + for metadata_field in CorporaConstants.REQUIRED_JSON_ENCODED_METADATA_FIELD: + uns[metadata_field] = json.dumps({"random_key": "random_value"}) + + # Need to carefully set the corpora schema versions in order for tests to pass. + uns["version"] = {"corpora_schema_version": "1.0.0", "corpora_encoding_version": "0.1.0"} + + # Set project links to be a dictionary + uns["project_links"] = json.dumps( + [{"link_name": "random_link_name", "link_url": "www.link.com", "link_type": "SUMMARY"}]) + + return anndata.AnnData(X=X, obs=obs, var=var, obsm=obsm, uns=uns) From 053f39d49eea602871161f248e2385e7531c433f Mon Sep 17 00:00:00 2001 From: maniarathi Date: Mon, 17 Aug 2020 18:40:26 -0700 Subject: [PATCH 05/15] Cleaning up one script that makes use of the non-existent cxgtool. (#1765) --- server/converters/to_sparse.py | 29 ++++++++++++++++++++++++----- 1 file changed, 24 insertions(+), 5 deletions(-) diff --git a/server/converters/to_sparse.py b/server/converters/to_sparse.py index 69602526..3603240a 100644 --- a/server/converters/to_sparse.py +++ b/server/converters/to_sparse.py @@ -2,12 +2,15 @@ Script to create a sparse dataset in CXG format based on an input dataset in CXG format. The input dataset is not modified. """ +import argparse import os import shutil -import tiledb -import argparse import sys -import server.converters.cxgtool as cxgtool + +import tiledb + +from server.common.utils.cxg_generation_utils import convert_ndarray_to_cxg_dense_array, convert_matrix_to_cxg_array +from server.common.utils.matrix_utils import is_matrix_sparse, get_column_shift_encode_for_matrix def main(): @@ -49,9 +52,25 @@ def main(): ) with tiledb.DenseArray(os.path.join(args.input, "X"), mode="r", ctx=ctx) as X_in: - is_sparse = cxgtool.save_X(args.output, X_in, ctx, args.sparse_threshold, expect_sparse=True) + x_matrix_data = X_in[:, :] + matrix_container = args.output - if is_sparse is False: + is_sparse = is_matrix_sparse(x_matrix_data, args.sparse_threshold) + if not is_sparse: + col_shift = get_column_shift_encode_for_matrix(x_matrix_data, args.sparse_threshold) + is_sparse = col_shift is not None + else: + col_shift = None + + if col_shift is not None: + x_col_shift_name = f"{args.output}/X_col_shift" + convert_ndarray_to_cxg_dense_array(x_col_shift_name, col_shift, ctx) + tiledb.consolidate(matrix_container, ctx=ctx) + if is_sparse: + convert_matrix_to_cxg_array(matrix_container, x_matrix_data, is_sparse, ctx, col_shift) + tiledb.consolidate(matrix_container, ctx=ctx) + + if not is_sparse: print("The array is not sparse, cleaning up, abort.") shutil.rmtree(args.output) sys.exit(1) From 950be4426d1d05ddf59e088bf47a507545c8114e Mon Sep 17 00:00:00 2001 From: bmccandless Date: Tue, 18 Aug 2020 14:41:15 -0700 Subject: [PATCH 06/15] Handle the refresh token in oauth authentication (#1766) * Handle the refresh token in oauth authentication If the token has expired, then it can be refreshed to get a new token. This is automatically handled by the server without the client being aware. Also in the PR: - refactor the auth_oauth.py file to more simply handle the save/restore of the token, and the refresh token - added an end2end test for oauth, which also tests refresh. * adding python-jose and Authlib to requirements-dev.txt They are needed in the auth_oauth test --- server/auth/auth_oauth.py | 253 +++++++++++++++++++--------- server/requirements-dev.txt | 10 +- server/test/unit/auth/test_oauth.py | 176 +++++++++++++++++++ 3 files changed, 356 insertions(+), 83 deletions(-) create mode 100644 server/test/unit/auth/test_oauth.py diff --git a/server/auth/auth_oauth.py b/server/auth/auth_oauth.py index 7f35d43f..084d1af9 100644 --- a/server/auth/auth_oauth.py +++ b/server/auth/auth_oauth.py @@ -1,9 +1,10 @@ -from flask import session, request, redirect, current_app, has_request_context, g +from flask import session, request, redirect, current_app, after_this_request, has_request_context, g from server.auth.auth import AuthTypeClientBase, AuthTypeFactory from server.common.errors import AuthenticationError, ConfigurationError from urllib.parse import urlencode -from urllib.request import urlopen import json +import requests +import base64 # It is not required to have authlib or jose. # However, it is a configuration error to use this auth type if they are not installed. @@ -20,10 +21,22 @@ except ModuleNotFoundError: missingimport.append("jose") +class Tokens: + """Simple class to represent the tokens that are saved/restored from the cookie""" + + def __init__(self, access_token, id_token, refresh_token, expires_at): + self.access_token = access_token + self.id_token = id_token + self.refresh_token = refresh_token + self.expires_at = expires_at + if not (access_token and id_token and refresh_token and expires_at): + raise KeyError(str(self.__dict__)) + + class AuthTypeOAuth(AuthTypeClientBase): """An authentication type for oauth2 logins.""" - CXG_ID_TOKEN = "id_token" + CXG_TOKENS = "auth_tokens" def __init__(self, server_config): super().__init__() @@ -46,8 +59,8 @@ class AuthTypeOAuth(AuthTypeClientBase): # any JSON Web Token (JWT) issued by the authorization server and signed using the RS256 try: jwksloc = f"{self.api_base_url}/.well-known/jwks.json" - jwksurl = urlopen(jwksloc) - self.jwks = json.loads(jwksurl.read()) + jwksurl = requests.get(jwksloc) + self.jwks = jwksurl.json() except Exception: raise ConfigurationError(f"error in oauth, api_url_base: {self.api_base_url}, cannot access {jwksloc}") @@ -87,48 +100,43 @@ class AuthTypeOAuth(AuthTypeClientBase): self.callback_base_url = f"http://{server_config.app__host}:{server_config.app__port}" self.client = self.oauth.register( - "oauth", + "auth0", client_id=self.client_id, client_secret=self.client_secret, api_base_url=self.api_base_url, + refresh_token_url=f"{self.api_base_url}/oauth/token", access_token_url=f"{self.api_base_url}/oauth/token", authorize_url=f"{self.api_base_url}/authorize", - client_kwargs={ - "scope" : "openid profile email", - } + client_kwargs={"scope": "openid profile email offline_access"}, ) def is_user_authenticated(self): - try: - payload = self.get_jwt_payload() - return payload is not None - except AuthenticationError: - return False + payload = self.get_userinfo() + return payload is not None def get_user_id(self): - payload = self.get_jwt_payload() + payload = self.get_userinfo() if payload and payload.get("sub"): return payload.get("sub") return None def get_user_name(self): - payload = self.get_jwt_payload() + payload = self.get_userinfo() if payload and payload.get("name"): return payload.get("name") return None def get_user_email(self): - payload = self.get_jwt_payload() + payload = self.get_userinfo() if payload and payload.get("email"): return payload.get("email") return None def update_response(self, response): - response.cache_control.update( - dict(public=True, max_age=0, no_store=True, no_cache=True, must_revalidate=True)) + response.cache_control.update(dict(public=True, max_age=0, no_store=True, no_cache=True, must_revalidate=True)) def login(self): - callbackurl = f'{self.callback_base_url}/oauth2/callback' + callbackurl = f"{self.callback_base_url}/oauth2/callback" return_path = request.args.get("dataset", "") return_to = f"{self.callback_base_url}/{return_path}" # save the return path in the session cookie, accessed in the callback function @@ -138,40 +146,83 @@ class AuthTypeOAuth(AuthTypeClientBase): return response def logout(self): - params = {'returnTo' : self.callback_base_url, 'client_id' : self.client_id} - response = redirect(self.client.api_base_url + '/v2/logout?' + urlencode(params)) - - if self.session_cookie: - if self.CXG_ID_TOKEN in session: - del session[self.CXG_ID_TOKEN] - else: - response.set_cookie(self.cookie_params["key"], "", expires=0) - + self.remove_tokens() + params = {"returnTo": self.callback_base_url, "client_id": self.client_id} + response = redirect(self.client.api_base_url + "/v2/logout?" + urlencode(params)) self.update_response(response) return response def callback(self): - token = self.client.authorize_access_token() - id_token = token.get("id_token") + data = self.client.authorize_access_token() + tokens = Tokens( + access_token=data.get("access_token"), + id_token=data.get("id_token"), + refresh_token=data.get("refresh_token"), + expires_at=data.get("expires_at"), + ) + self.save_tokens(tokens) oauth_callback_redirect = session.pop("oauth_callback_redirect", "/") - resp = redirect(oauth_callback_redirect) + response = redirect(oauth_callback_redirect) + self.update_response(response) + return response + def get_tokens(self): + """Extract the tokens from the cookie, and store them in the flask global context""" + if "tokens" in g: + return g.tokens + + try: + if self.session_cookie: + tokensdict = session.get(self.CXG_TOKENS) + if tokensdict: + g.tokens = Tokens(**tokensdict) + else: + return None + else: + value = request.cookies.get(self.cookie_params["key"]) + value = base64.b64decode(value) + try: + tokensdict = json.loads(value) + g.tokens = Tokens(**tokensdict) + except (TypeError, KeyError, json.decoder.JSONDecodeError): + g.pop("tokens", None) + return None + + except (TypeError, KeyError): + g.pop("tokens", None) + return None + + return g.tokens + + def save_tokens(self, tokens): + g.tokens = tokens if self.session_cookie: - session[self.CXG_ID_TOKEN] = id_token + session[self.CXG_TOKENS] = tokens.__dict__ else: - args = self.cookie_params.copy() - del args["key"] - try: - resp.set_cookie( - self.cookie_params["key"], - id_token, - **args) - g.token = id_token - except Exception as e: - raise AuthenticationError(f"unable to set_cookie {self.cookie_params}") from e - self.update_response(resp) - return resp + @after_this_request + def set_cookie(response): + args = self.cookie_params.copy() + value = base64.b64encode(json.dumps(tokens.__dict__).encode("utf-8")) + del args["key"] + try: + response.set_cookie(self.cookie_params["key"], value, **args) + except Exception as e: + raise AuthenticationError(f"unable to set_cookie {self.cookie_params}") from e + return response + + def remove_tokens(self): + g.pop("tokens", None) + if self.session_cookie: + if self.CXG_TOKENS in session: + del session[self.CXG_TOKENS] + else: + + @after_this_request + def remove_cookie(response): + response.set_cookie(self.cookie_params["key"], "", expires=0) + self.update_response(response) + return response def get_login_url(self, data_adaptor): """Return the url for the login route""" @@ -184,60 +235,104 @@ class AuthTypeOAuth(AuthTypeClientBase): """Return the url for the logout route""" return "/logout" - def get_token(self): - """Function to return the token""" - if "token" in g: - return g.token - if self.session_cookie: - g.token = session.get(self.CXG_ID_TOKEN) - else: - g.token = request.cookies.get(self.cookie_params["key"]) - - return g.token - - def get_jwt_payload(self): - if not has_request_context(): - return None - - token = self.get_token() - if token is None: - return None - + def check_jwt_payload(self, id_token): try: - unverified_header = jwt.get_unverified_header(token) + unverified_header = jwt.get_unverified_header(id_token) except JWTError: return None rsa_key = {} - for key in self.jwks['keys']: - if key['kid'] == unverified_header['kid']: + for key in self.jwks["keys"]: + if key["kid"] == unverified_header["kid"]: rsa_key = { - 'kty': key['kty'], - 'kid': key['kid'], - 'use': key['use'], - 'n': key['n'], - 'e': key['e'] + "kty": key["kty"], + "kid": key["kid"], + "use": key["use"], + "n": key.get("n"), + "e": key.get("e"), } if rsa_key: + options = {} + if not rsa_key["n"] or not rsa_key["e"]: + # this is a mock auth server, do not validate + options = {"verify_signature": False, "verify_iss": False} try: payload = jwt.decode( - token, + id_token, rsa_key, algorithms=self.algorithms, audience=self.audience, - issuer=self.api_base_url + "/" + issuer=self.api_base_url + "/", + options=options, ) return payload - except JWTError as e: - raise AuthenticationError(f"invalid signature: {str(e)}") except ExpiredSignatureError: - # TODO, handle expired sessions by refreshing the token - return None + # This exception is handled in get_userinfo + raise except JWTClaimsError as e: - raise AuthenticationError(f"invalid claims {str(e)}") + raise AuthenticationError(f"invalid claims {str(e)}") from e + except JWTError as e: + raise AuthenticationError(f"invalid signature: {str(e)}") from e raise AuthenticationError("Unable to find the appropriate key") + def get_userinfo(self): + if not has_request_context(): + return None + + # check if the userinfo has been retrieved already in this request + if "userinfo" in g: + return g.get("userinfo") + + # if there is no id_token, return None (user is not authenticated) + tokens = self.get_tokens() + if tokens is None or tokens.id_token is None: + return None + + try: + # check the jwt payload. This raises an AuthenticationError if the token is not valid. + # It the token has expired, we attempt to refresh the token + g.userinfo = self.check_jwt_payload(tokens.id_token) + return g.userinfo + + except ExpiredSignatureError: + tokens = self.refresh_expired_token(tokens.refresh_token) + if tokens is None or tokens.id_token is None: + return None + else: + try: + g.userinfo = self.check_jwt_payload(tokens.id_token) + return g.userinfo + except JWTError as e: + raise AuthenticationError(f"error during token refresh: {str(e)}") from e + + except AuthenticationError: + self.remove_tokens() + raise + + def refresh_expired_token(self, refresh_token): + params = { + "grant_type": "refresh_token", + "client_id": self.client_id, + "refresh_token": refresh_token, + "client_secret": self.client_secret, + } + headers = {"content-type": "application/x-www-form-urlencoded"} + request = requests.post(f"{self.api_base_url}/oauth/token", urlencode(params), headers=headers) + if request.status_code != 200: + # unable to refresh the token, log the user out + self.remove_tokens() + return None + data = request.json() + tokens = Tokens( + access_token=data.get("access_token"), + id_token=data.get("id_token"), + refresh_token=data.get("refresh_token", refresh_token), + expires_at=data.get("expires_at"), + ) + self.save_tokens(tokens) + return tokens + AuthTypeFactory.register("oauth", AuthTypeOAuth) diff --git a/server/requirements-dev.txt b/server/requirements-dev.txt index 7921926f..0fb57ce8 100644 --- a/server/requirements-dev.txt +++ b/server/requirements-dev.txt @@ -1,9 +1,11 @@ +Authlib>=0.14.3 black bumpversion>=0.5 -parameterized>=0.7.0 -pytest>=3.6.3 -twine>=1.12.1 codecov>=2.0.15 -scanpy>=1.4.6 +parameterized>=0.7.0 psycopg2==2.7.7 +pytest>=3.6.3 +python-jose>=3.2.0 +scanpy>=1.4.6 +twine>=1.12.1 -r requirements.txt diff --git a/server/test/unit/auth/test_oauth.py b/server/test/unit/auth/test_oauth.py new file mode 100644 index 00000000..413a1084 --- /dev/null +++ b/server/test/unit/auth/test_oauth.py @@ -0,0 +1,176 @@ +import unittest +import random +import time +import base64 +import json +import requests + +from flask import Flask, jsonify, make_response, request, redirect +from multiprocessing import Process + +import jose +from server.common.app_config import AppConfig +from server.test import FIXTURES_ROOT, test_server + +# This tests the oauth authentication type. +# This test starts a cellxgene server and a mock oauth server. +# API requests to login and logout and get the userinfo are made +# to the cellxgene server, which then sends requests to the mock +# oauth server. + +# number of seconds that the oauth token is valid +TOKEN_EXPIRES = 5 + +# Create a mocked out oauth token, which servers all the endpoints needed by the oauth type. +mock_oauth_app = Flask("mock_oauth_app") + + +@mock_oauth_app.route("/authorize") +def authorize(): + callback = request.args.get("redirect_uri") + state = request.args.get("state") + return redirect(callback + f"?code=fakecode&state={state}") + + +@mock_oauth_app.route("/oauth/token", methods=["POST"]) +def token(): + headers = dict(alg="RS256", kid="fake_kid") + payload = dict(name="fake_user", sub="fake_id", email="fake_user@email.com", email_verified=True) + jwt = jose.jwt.encode(claims=payload, key="mysecret", algorithm="HS256", headers=headers) + r = { + "access_token": f"access-{time.time()}", + "id_token": jwt, + "refresh_token": f"random-{time.time()}", + "scope": "openid profile email", + "expires_in": TOKEN_EXPIRES, + "token_type": "Bearer", + "expires_at": time.time() + TOKEN_EXPIRES, + } + return make_response(jsonify(r)) + + +@mock_oauth_app.route("/v2/logout") +def logout(): + return_to = request.args.get("returnTo") + return redirect(return_to) + + +@mock_oauth_app.route("/.well-known/jwks.json") +def jwks(): + data = dict(alg="RS256", kty="RSA", use="sig", kid="fake_kid",) + return make_response(jsonify(dict(keys=[data]))) + + +# The port that the mock oauth server will listen on +PORT = random.randint(10000, 12000) + + +# function to launch the mock oauth server +def launch_mock_oauth(): + mock_oauth_app.run(port=PORT) + + +class AuthTest(unittest.TestCase): + def setUp(self): + self.dataset_dataroot = FIXTURES_ROOT + self.mock_oauth_process = Process(target=launch_mock_oauth) + self.mock_oauth_process.start() + + def tearDown(self): + self.mock_oauth_process.terminate() + + def auth_flow(self, app_config, cookie_key=None): + + with test_server(app_config=app_config) as server: + session = requests.Session() + + # auth datasets + config = session.get(f"{server}/d/pbmc3k.cxg/api/v0.2/config").json() + userinfo = session.get(f"{server}/d/pbmc3k.cxg/api/v0.2/userinfo").json() + + self.assertFalse(userinfo["userinfo"]["is_authenticated"]) + self.assertIsNone(userinfo["userinfo"]["username"]) + self.assertTrue(config["config"]["authentication"]["requires_client_login"]) + self.assertTrue(config["config"]["parameters"]["annotations"]) + + login_uri = config["config"]["authentication"]["login"] + logout_uri = config["config"]["authentication"]["logout"] + + self.assertEqual(login_uri, "/login?dataset=d/pbmc3k.cxg/") + self.assertEqual(logout_uri, "/logout") + + r = session.get(f"{server}/{login_uri}") + # check that the login redirect worked + self.assertEqual(r.history[0].status_code, 302) + self.assertEqual(r.url, f"{server}/d/pbmc3k.cxg/") + config = session.get(f"{server}/d/pbmc3k.cxg/api/v0.2/config").json() + userinfo = session.get(f"{server}/d/pbmc3k.cxg/api/v0.2/userinfo").json() + self.assertTrue(userinfo["userinfo"]["is_authenticated"]) + self.assertEqual(userinfo["userinfo"]["username"], "fake_user") + self.assertTrue(config["config"]["parameters"]["annotations"]) + + if cookie_key: + cookie = session.cookies.get(cookie_key) + token = json.loads(base64.b64decode(cookie)) + access_token_before = token.get("access_token") + expires_at_before = token.get("expires_at") + + # let the token expire + time.sleep(TOKEN_EXPIRES + 1) + + # check that refresh works + session.get(f"{server}/{login_uri}") + userinfo = session.get(f"{server}/d/pbmc3k.cxg/api/v0.2/userinfo").json() + self.assertTrue(userinfo["userinfo"]["is_authenticated"]) + self.assertEqual(userinfo["userinfo"]["username"], "fake_user") + + cookie = session.cookies.get(cookie_key) + token = json.loads(base64.b64decode(cookie)) + access_token_after = token.get("access_token") + expires_at_after = token.get("expires_at") + + self.assertNotEqual(access_token_before, access_token_after) + self.assertTrue(expires_at_after - expires_at_before > TOKEN_EXPIRES) + + r = session.get(f"{server}/{logout_uri}") + # check that the logout redirect worked + self.assertEqual(r.history[0].status_code, 302) + self.assertEqual(r.url, f"{server}") + config = session.get(f"{server}/d/pbmc3k.cxg/api/v0.2/config").json() + userinfo = session.get(f"{server}/d/pbmc3k.cxg/api/v0.2/userinfo").json() + self.assertFalse(userinfo["userinfo"]["is_authenticated"]) + self.assertIsNone(userinfo["userinfo"]["username"]) + self.assertTrue(config["config"]["parameters"]["annotations"]) + + def test_auth_oauth_session(self): + # test with session cookies + app_config = AppConfig() + app_config.update_server_config( + authentication__type="oauth", + authentication__params_oauth__api_base_url=f"http://localhost:{PORT}", + authentication__params_oauth__client_id="mock_client_id", + authentication__params_oauth__client_secret="mock_client_secret", + authentication__params_oauth__session_cookie=True, + ) + + app_config.update_server_config(multi_dataset__dataroot=self.dataset_dataroot) + app_config.complete_config() + + self.auth_flow(app_config) + + def test_auth_oauth_cookie(self): + # test with specified cookie + app_config = AppConfig() + app_config.update_server_config( + authentication__type="oauth", + authentication__params_oauth__api_base_url=f"http://localhost:{PORT}", + authentication__params_oauth__client_id="mock_client_id", + authentication__params_oauth__client_secret="mock_client_secret", + authentication__params_oauth__session_cookie=False, + authentication__params_oauth__cookie=dict(key="test_cxguser", httponly=True, max_age=60), + ) + + app_config.update_server_config(multi_dataset__dataroot=self.dataset_dataroot) + app_config.complete_config() + + self.auth_flow(app_config, "test_cxguser") From 924aaf9aef6b68359bbec954525e351f58c814c7 Mon Sep 17 00:00:00 2001 From: bmccandless Date: Tue, 18 Aug 2020 17:13:57 -0700 Subject: [PATCH 07/15] Allow user_annotations in the eb app (#1781) --- server/eb/app.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/eb/app.py b/server/eb/app.py index 9aa86263..159e6dc4 100644 --- a/server/eb/app.py +++ b/server/eb/app.py @@ -158,7 +158,7 @@ try: # features are unsupported in the current hosted server app_config.update_default_dataset_config( - user_annotations__enable=False, embeddings__enable_reembedding=False, + embeddings__enable_reembedding=False, ) app_config.update_server_config(multi_dataset__allowed_matrix_types=["cxg"],) app_config.complete_config(logging.info) From fae9ac93821317830e259296a13a169cdfb50d11 Mon Sep 17 00:00:00 2001 From: Snyk bot Date: Thu, 20 Aug 2020 21:03:30 +0300 Subject: [PATCH 08/15] Upgrade lodash from 4.17.15 to 4.17.20 (#1759) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit

Snyk has created this PR to fix one or more vulnerable packages in the `npm` dependencies of this project.

![merge advice](https://app.snyk.io/badges/merge-advice/?package_manager=npm&package_name=lodash&from_version=4.17.15&to_version=4.17.20&pr_id=31842747-752f-42e7-b1f2-8fa3f51d0e21&visibility=true&has_feature_flag=false) #### Changes included in this PR - Changes to the following files to upgrade the vulnerable dependencies to a fixed version: - client/package.json - client/package-lock.json #### Vulnerabilities that will be fixed ##### With an upgrade: Severity | Priority Score (*) | Issue | Breaking Change | Exploit Maturity :-------------------------:|-------------------------|:-------------------------|:-------------------------|:------------------------- ![high severity](https://res.cloudinary.com/snyk/image/upload/w_20,h_20/v1561977819/icon/h.png "high severity") | **776/1000**
**Why?** Recently disclosed, Has a fix available, CVSS 9.8 | Prototype Pollution
[SNYK-JS-LODASH-590103](https://snyk.io/vuln/SNYK-JS-LODASH-590103) | No | No Known Exploit (*) Note that the real score may have changed since the PR was raised. Check the changes in this PR to ensure they won't cause issues with your project. ------------ **Note:** *You are seeing this because you or someone else with access to this repository has authorized Snyk to open fix PRs.* For more information: 🧐 [View latest project report](https://app.snyk.io/org/cellxgene/project/9195ddb9-6feb-469e-ad47-f5dc24c811fe) 🛠 [Adjust project settings](https://app.snyk.io/org/cellxgene/project/9195ddb9-6feb-469e-ad47-f5dc24c811fe/settings) 📚 [Read more about Snyk's upgrade and patch logic](https://support.snyk.io/hc/en-us/articles/360003891078-Snyk-patches-to-fix-vulnerabilities) [//]: # (snyk:metadata:{"prId":"31842747-752f-42e7-b1f2-8fa3f51d0e21","dependencies":[{"name":"lodash","from":"4.17.15","to":"4.17.20"}],"packageManager":"npm","projectPublicId":"9195ddb9-6feb-469e-ad47-f5dc24c811fe","projectUrl":"https://app.snyk.io/org/cellxgene/project/9195ddb9-6feb-469e-ad47-f5dc24c811fe?utm_source=github&utm_medium=fix-pr","type":"auto","patch":[],"vulns":["SNYK-JS-LODASH-590103"],"upgrade":["SNYK-JS-LODASH-590103"],"isBreakingChange":false,"env":"prod","prType":"fix","templateVariants":["updated-fix-title","priorityScore","merge-advice-badge-shown"],"priorityScoreList":[776]}) --- client/package-lock.json | 6 +++--- client/package.json | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/client/package-lock.json b/client/package-lock.json index 9038fb82..c94ba503 100644 --- a/client/package-lock.json +++ b/client/package-lock.json @@ -13787,9 +13787,9 @@ } }, "lodash": { - "version": "4.17.19", - "resolved": "https://registry.npmjs.org/lodash/-/lodash-4.17.19.tgz", - "integrity": "sha512-JNvd8XER9GQX0v2qJgsaN/mzFCNA5BRe/j8JN9d+tWyGLSodKQHKFicdwNYzWwI3wjRnaKPsGj1XkBjx/F96DQ==" + "version": "4.17.20", + "resolved": "https://registry.npmjs.org/lodash/-/lodash-4.17.20.tgz", + "integrity": "sha512-PlhdFcillOINfeV7Ni6oF1TAEayyZBoZ8bcshTHqOYJYlrqzRK5hagpagky5o4HfCzzd1TRkXPMFq6cKk9rGmA==" }, "lodash._reinterpolate": { "version": "3.0.0", diff --git a/client/package.json b/client/package.json index aa4892f9..9e6f20ef 100644 --- a/client/package.json +++ b/client/package.json @@ -52,7 +52,7 @@ "gl-matrix": "^3.3.0", "gl-vec3": "^1.1.3", "is-number": "^7.0.0", - "lodash": "^4.17.19", + "lodash": "^4.17.20", "memoize-one": "^5.1.1", "react": "^16.13.1", "react-async": "^10.0.1", From a5c9ffa88075aeb505298c353650a5b176565fbf Mon Sep 17 00:00:00 2001 From: maniarathi Date: Sat, 22 Aug 2020 09:42:11 -0700 Subject: [PATCH 09/15] When reading annotations from tiledb, check if the values are byte literals and if so, decode them. Also pin s3f3 to 0.4.2. (#1788) --- server/common/annotations/hosted_tiledb.py | 4 ++++ server/requirements.txt | 2 +- .../test/unit/common/test_writable_annotation.py | 15 ++++++--------- 3 files changed, 11 insertions(+), 10 deletions(-) diff --git a/server/common/annotations/hosted_tiledb.py b/server/common/annotations/hosted_tiledb.py index 16ac7b29..f65a6654 100644 --- a/server/common/annotations/hosted_tiledb.py +++ b/server/common/annotations/hosted_tiledb.py @@ -74,6 +74,10 @@ class AnnotationsHostedTileDB(Annotations): indexes = list() for col_name, col_val in data.items(): + # If the column values are byte literals, decode them + 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: new_col = pd.Series(col_val, dtype=repr_meta[col_name]) data[col_name] = new_col diff --git a/server/requirements.txt b/server/requirements.txt index ac08deed..d188764b 100644 --- a/server/requirements.txt +++ b/server/requirements.txt @@ -20,5 +20,5 @@ scipy>=1.3.0 requests>=2.22.0 sqlalchemy>=1.3.18 tiledb>=0.5.9,>=0.6.2 -s3fs>=0.4.2 +s3fs==0.4.2 gunicorn>=20.0.4 diff --git a/server/test/unit/common/test_writable_annotation.py b/server/test/unit/common/test_writable_annotation.py index 45e09de3..a13c7569 100644 --- a/server/test/unit/common/test_writable_annotation.py +++ b/server/test/unit/common/test_writable_annotation.py @@ -1,22 +1,20 @@ import json -from os import path, listdir +import shutil import unittest +from os import path, listdir from unittest.mock import MagicMock, patch +import numpy as np +import pandas as pd import tiledb from flask import Flask import server.test.unit.decode_fbs as decode_fbs -import shutil - -import numpy as np -import pandas as pd - +from server.common.errors import AnnotationCategoryNameError from server.common.rest import schema_get_helper, annotations_put_fbs_helper +from server.data_common.matrix_loader import MatrixDataType from server.db.cellxgene_orm import CellxGeneDataset, Annotation from server.test import data_with_tmp_annotations, make_fbs, data_with_tmp_tiledb_annotations -from server.data_common.matrix_loader import MatrixDataType -from server.common.errors import AnnotationCategoryNameError class auth(object): @@ -77,7 +75,6 @@ class WritableTileDBStoredAnnotationTest(unittest.TestCase): def test_write_labels_creates_a_dataset_if_it_doesnt_exist(self): with self.app.test_request_context(): - new_name = 'new_dataset/location' self.data.get_location = MagicMock(return_value=new_name) num_datasets = len(self.db.query([CellxGeneDataset])) From bc150a84698cf7a28900df865ad9daea85dc0c1c Mon Sep 17 00:00:00 2001 From: maniarathi Date: Sat, 22 Aug 2020 09:53:59 -0700 Subject: [PATCH 10/15] Fixing bugs in cxg conversion tool (#1782) --- server/cli/cli.py | 4 +- server/cli/convert_to_cxg.py | 20 ++- server/common/utils/cxg_generation_utils.py | 1 - server/common/utils/type_conversion_utils.py | 65 ++++++++-- server/data_common/fbs/matrix.py | 2 +- server/data_cxg/cxg_adaptor.py | 51 ++------ .../utils/test_type_conversion_utils.py | 118 ++++++++++++++---- 7 files changed, 174 insertions(+), 87 deletions(-) diff --git a/server/cli/cli.py b/server/cli/cli.py index 9bb9105a..dc3e5837 100644 --- a/server/cli/cli.py +++ b/server/cli/cli.py @@ -1,9 +1,10 @@ import click -from .. import __version__ +from .convert_to_cxg import convert_to_cxg from .launch import launch from .prepare import prepare from .upgrade import log_upgrade_check +from .. import __version__ @click.group( @@ -29,3 +30,4 @@ def cli(upgrade_check): cli.add_command(launch) cli.add_command(prepare) +cli.add_command(convert_to_cxg) diff --git a/server/cli/convert_to_cxg.py b/server/cli/convert_to_cxg.py index e8d37936..4e456c08 100644 --- a/server/cli/convert_to_cxg.py +++ b/server/cli/convert_to_cxg.py @@ -16,12 +16,11 @@ from server.converters.h5ad_data_file import H5ADDataFile @click.argument( "input-file", nargs=1, - help="Path to the H5AD input file to be converted.", type=click.Path(exists=True, dir_okay=False), ) @click.option( "-o", - "--output-dir", + "--output-directory", help="Name of the output CXG directory. If not provided, will default to be the input filename with a " "CXG extension.", ) @@ -70,9 +69,9 @@ from server.converters.h5ad_data_file import H5ADDataFile ) @click.option( "--disable-corpora-schema", - "When set, conversion process will neither extract nor store Corpora schema information. See " - "https://github.com/chanzuckerberg/corpora-data-portal/blob/main/backend/schema/corpora_schema.md for " - "more information.", + help="When set, conversion process will neither extract nor store Corpora schema information. See " + "https://github.com/chanzuckerberg/corpora-data-portal/blob/main/backend/schema/corpora_schema.md for more " + "information.", default=False, show_default=True, is_flag=True, @@ -84,7 +83,6 @@ from server.converters.h5ad_data_file import H5ADDataFile show_default=True, is_flag=True, ) -@click.option("-v", "--verbose", count=True) @click.help_option("--help", "-h", help="Show this message and exit.") def convert_to_cxg( input_file, @@ -97,7 +95,7 @@ def convert_to_cxg( var_names, disable_custom_colors, disable_corpora_schema, - should_overwrite, + overwrite, ): """ Convert a dataset file into CXG. @@ -107,7 +105,7 @@ def convert_to_cxg( use_corpora_schema=not disable_corpora_schema) # Get the directory that will hold all the CXG files - cxg_output_container = get_output_directory(input_file, output_directory, should_overwrite) + cxg_output_container = get_output_directory(input_file, output_directory, overwrite) h5ad_data_file.to_cxg(cxg_output_container, sparse_threshold, convert_anndata_colors_to_cxg_colors=not disable_custom_colors) @@ -118,14 +116,14 @@ def get_output_directory(input_filename, output_directory, should_overwrite): Get the name of the CXG output directory to be created/populated during the dataset conversion. """ - if not path.isdir(output_directory) or (path.isdir(output_directory) and should_overwrite): + if output_directory and (not path.isdir(output_directory) or (path.isdir(output_directory) and should_overwrite)): if output_directory.endswith(".cxg"): return output_directory return output_directory + ".cxg" - if path.isdir(output_directory) and not should_overwrite: + if output_directory and path.isdir(output_directory) and not should_overwrite: raise click.BadParameter( f"Output directory {output_directory} already exists. If you'd like to overwrite, then run the command " f"with the --overwrite flag." ) - return path.splitext(input_filename)[1] + ".cxg" + return path.splitext(input_filename)[0] + ".cxg" diff --git a/server/common/utils/cxg_generation_utils.py b/server/common/utils/cxg_generation_utils.py index f336c0ae..f29f4bd0 100644 --- a/server/common/utils/cxg_generation_utils.py +++ b/server/common/utils/cxg_generation_utils.py @@ -67,7 +67,6 @@ def convert_dataframe_to_cxg_array(cxg_container, dataframe_name, dataframe, ind schema_hints = {} for column_name, column_values in dataframe.items(): dtype, hints = get_dtype_and_schema_of_array(column_values) - value[column_name] = column_values.to_numpy(dtype=dtype) if hints: schema_hints.update({column_name: hints}) diff --git a/server/common/utils/type_conversion_utils.py b/server/common/utils/type_conversion_utils.py index eca8b938..ac6b3fe4 100644 --- a/server/common/utils/type_conversion_utils.py +++ b/server/common/utils/type_conversion_utils.py @@ -37,19 +37,19 @@ def get_dtype_from_dtype(dtype, array_values=None): dtype_name = dtype.name dtype_kind = dtype.kind - if dtype == np.float32 or dtype == np.int32: - return dtype if dtype_name == "bool": return np.uint8 if dtype_name == "object" and dtype_kind == "O": return np.unicode if dtype_name == "category": - return get_dtype_from_dtype(dtype.categories.dtype, dtype.categories) + return get_dtype_from_dtype(dtype.categories.dtype, array_values) - if can_cast_to_float32(dtype): - return np.float32 if can_cast_to_int32(dtype, array_values): return np.int32 + if can_cast_to_float32(dtype, array_values): + return np.float32 + if not can_cast_to_float32(dtype, array_values): + return np.float64 raise TypeError(f"Annotations of type {dtype} are unsupported.") @@ -72,19 +72,43 @@ def get_schema_type_hint_from_dtype(dtype, array_values=None): if dtype_name == "category": return {"type": "categorical", "categories": dtype.categories.tolist()} - if can_cast_to_float32(dtype): - return {"type": "float32"} if can_cast_to_int32(dtype, array_values): return {"type": "int32"} + if can_cast_to_float32(dtype, array_values): + return {"type": "float32"} + if dtype_kind == "f" and not can_cast_to_float32(dtype, array_values): + return {"type": "float64"} raise TypeError(f"Annotations of type {dtype} are unsupported.") -def can_cast_to_float32(dtype): +def can_cast_to_float32(dtype, array_values): + """ + A dtype can be cast to float32 if it is a float type and converting it to float32 presents the same output as the + original values. Note that NaNs fail equality (i.e. np.NaN != np.NaN) so we use np.testing.assert_equal to ensure + that the arrays are equal minus NaNs. + + We also handle a special case here where the array is a Series object with integer categorical values AND NaNs. + Since NaNs are floating points in numpy, we upcast the integer array to float32. + """ + if dtype.kind == "f": - if not np.can_cast(dtype, np.float32): + # Try to convert the array to float32 + converted_float32_values = array_values.to_numpy(np.float32) + original_values = array_values.to_numpy() + + # Verify that the two arrays are equal except for NaNs (which will equate to be unequal). + if not ((converted_float32_values != original_values) == np.isnan(original_values)).all(): + return False + + if dtype != np.float32: logging.warning(f"Type {dtype.name} will be converted to 32 bit float and may lose precision.") + return True + + if dtype.kind == "O" and array_values.hasnans: + return True + return False @@ -94,11 +118,30 @@ def can_cast_to_int32(dtype, array_values=None): the higher precision type has values that are entirely within the range of the downcast type. """ + # Since a NaN is technically a float, any array that contains NaNs cannot be cast to an integer so immediately + # return False. + if array_values.hasnans: + return False + + # If the array is categorical, then we need to order the array values so that functions min and max that occur + # later, can function. They do not function on unordered categories. + ordered_array_values = array_values + if array_values.dtype.name == "category" and not array_values.cat.ordered: + ordered_array_values = array_values.cat.as_ordered() + if dtype.kind in ["i", "u"]: if np.can_cast(dtype, np.int32): return True ii32 = np.iinfo(np.int32) - if not array_values.empty and ( - array_values.min() >= ii32.min and array_values.max() <= ii32.max) or array_values.empty: + if not ordered_array_values.empty and ( + ordered_array_values.min() >= ii32.min and ordered_array_values.max() <= ii32.max) or \ + ordered_array_values.empty: return True return False + + +def convert_pandas_series_to_numpy(series_to_convert: pd.Series, dtype): + if series_to_convert.hasnans and dtype == np.int32: + logging.error("Cannot convert a pandas Series object to an integer dtype if it contains NaNs.") + + return series_to_convert.to_numpy(dtype) diff --git a/server/data_common/fbs/matrix.py b/server/data_common/fbs/matrix.py index 213e3ce4..e4e33d33 100644 --- a/server/data_common/fbs/matrix.py +++ b/server/data_common/fbs/matrix.py @@ -85,7 +85,7 @@ def serialize_typed_array(builder, source_array, encoding_info): def column_encoding(arr): column_encoding_type_map = { # array protocol string: ( array_type, as_type ) - np.dtype(np.float64).str: (TypedArray.TypedArray.Float32Array, np.float32), + np.dtype(np.float64).str: (TypedArray.TypedArray.Float64Array, np.float64), np.dtype(np.float32).str: (TypedArray.TypedArray.Float32Array, np.float32), np.dtype(np.float16).str: (TypedArray.TypedArray.Float32Array, np.float32), np.dtype(np.int8).str: (TypedArray.TypedArray.Int32Array, np.int32), diff --git a/server/data_cxg/cxg_adaptor.py b/server/data_cxg/cxg_adaptor.py index db2fdd2a..3963eb68 100644 --- a/server/data_cxg/cxg_adaptor.py +++ b/server/data_cxg/cxg_adaptor.py @@ -1,24 +1,25 @@ -import os import json import logging -from server.common.utils.type_conversion_utils import get_schema_type_hint_from_dtype -from server.common.errors import DatasetAccessError, ConfigurationError -from server.common.utils.utils import path_join +import os +import threading + +import numpy as np +import pandas as pd +import tiledb +from server_timing import Timing as ServerTiming + +import server.compute.diffexp_cxg as diffexp_cxg from server.common.constants import Axis +from server.common.errors import DatasetAccessError, ConfigurationError +from server.common.immutable_kvcache import ImmutableKVCache +from server.common.utils.type_conversion_utils import get_schema_type_hint_from_dtype +from server.common.utils.utils import path_join from server.data_common.data_adaptor import DataAdaptor from server.data_common.fbs.matrix import encode_matrix_fbs from server.data_cxg.cxg_util import pack_selector_from_mask -import server.compute.diffexp_cxg as diffexp_cxg -from server.common.immutable_kvcache import ImmutableKVCache -import tiledb -import numpy as np -import pandas as pd -from server_timing import Timing as ServerTiming -import threading class CxgAdaptor(DataAdaptor): - # TODO: The tiledb context parameters should be a configuration option tiledb_ctx = tiledb.Ctx( {"sm.tile_cache_size": 8 * 1024 * 1024 * 1024, "sm.num_reader_threads": 32, "vfs.s3.region": "us-east-1"} @@ -337,32 +338,6 @@ class CxgAdaptor(DataAdaptor): raise DatasetAccessError("cxg matrix missing embeddings") return embeddings - @staticmethod - def _get_col_type(attr, schema_hints={}): - type_hint = schema_hints.get(attr.name, {}) - dtype = attr.dtype - schema = {} - # type hints take precedence - if "type" in type_hint: - schema["type"] = type_hint["type"] - elif dtype == np.float32: - schema["type"] = "float32" - elif dtype == np.int32: - schema["type"] = "int32" - elif dtype == np.bool_: - schema["type"] = "boolean" - elif dtype == np.str: - schema["type"] = "string" - elif dtype == "category": - schema["type"] = "categorical" - schema["categories"] = dtype.categories.tolist() - else: - raise TypeError(f"Annotations of type {dtype} are unsupported.") - - if schema["type"] == "categorical" and "categories" in schema_hints: - schema["categories"] = schema_hints["categories"] - return schema - def _get_schema(self): if self.schema: return self.schema diff --git a/server/test/unit/common/utils/test_type_conversion_utils.py b/server/test/unit/common/utils/test_type_conversion_utils.py index 9778bfca..f1653697 100644 --- a/server/test/unit/common/utils/test_type_conversion_utils.py +++ b/server/test/unit/common/utils/test_type_conversion_utils.py @@ -1,11 +1,12 @@ import unittest +from time import time from unittest.mock import patch import numpy as np from pandas import Series, DataFrame from server.common.utils.type_conversion_utils import can_cast_to_float32, can_cast_to_int32, get_dtype_of_array, \ - get_schema_type_hint_of_array, get_dtypes_and_schemas_of_dataframe + get_schema_type_hint_of_array, get_dtypes_and_schemas_of_dataframe, convert_pandas_series_to_numpy class TestTypeConversionUtils(unittest.TestCase): @@ -13,28 +14,49 @@ class TestTypeConversionUtils(unittest.TestCase): def test__can_cast_to_float32__string_is_false(self): array_to_convert = Series(data=["1", "2", "3"], dtype=str) - can_cast = can_cast_to_float32(array_to_convert.dtype) + can_cast = can_cast_to_float32(array_to_convert.dtype, array_to_convert) self.assertFalse(can_cast) - def test__can_cast_to_float32__int_is_true_warning_outputted(self): + def test__can_cast_to_float32__float64_is_true_warning_outputted(self): array_to_convert = Series(data=[1, 2, 3], dtype=np.dtype(np.float64)) with self.assertLogs(level="WARN") as logger: - can_cast = can_cast_to_float32(array_to_convert.dtype) + can_cast = can_cast_to_float32(array_to_convert.dtype, array_to_convert) self.assertIn("may lose precision", logger.output[0]) self.assertTrue(can_cast) @patch("logging.warning") - def test__can_cast_to_float64__int_is_false(self, mock_log_warning): + def test__can_cast_to_float32__float32_is_false(self, mock_log_warning): array_to_convert = Series(data=[1, 2, 3], dtype=np.dtype(np.float32)) - can_cast = can_cast_to_float32(array_to_convert.dtype) + can_cast = can_cast_to_float32(array_to_convert.dtype, array_to_convert) self.assertTrue(can_cast) assert not mock_log_warning.called + def test__can_cast_to_float32__categorical_float64_is_false(self): + array_to_convert = Series(data=[1.1, 2.2, 3.3], dtype="category") + + can_cast = can_cast_to_float32(array_to_convert.dtype, array_to_convert) + + self.assertFalse(can_cast) + + def test__can_cast_to_float32__categorical_int64_with_nans_is_true(self): + array_to_convert = Series(data=[1, 2, np.NaN], dtype="category") + + can_cast = can_cast_to_float32(array_to_convert.dtype, array_to_convert) + + self.assertTrue(can_cast) + + def test__can_cast_to_float_32__float_32_with_nans_is_true(self): + array_to_convert = Series(data=[1, 2, np.NaN], dtype=np.dtype(np.float32)) + + can_cast = can_cast_to_float32(array_to_convert.dtype, array_to_convert) + + self.assertTrue(can_cast) + def test__can_cast_to_int32__string_is_false(self): array_to_convert = Series(data=["1", "2", "3"], dtype=str) @@ -63,6 +85,13 @@ class TestTypeConversionUtils(unittest.TestCase): self.assertFalse(can_cast) + def test__can_cast_to_int32__int64_with_nans_is_false(self): + array_to_convert = Series(data=[np.NaN, "2", "3"], dtype="category") + + can_cast = can_cast_to_int32(array_to_convert.dtype, array_to_convert) + + self.assertFalse(can_cast) + def test__get_dtype_of_array__supported_dtypes_return_as_expected(self): types = [np.float32, np.int32, np.bool_, str] expected_dtypes = [np.float32, np.int32, np.uint8, np.unicode] @@ -73,6 +102,40 @@ class TestTypeConversionUtils(unittest.TestCase): array = Series(data=[], dtype=types[test_type_index]) self.assertEqual(get_dtype_of_array(array), expected_dtypes[test_type_index]) + def test__get_dtype_of_array__categories_return_as_expected(self): + array = Series(data=["a", "b", "c"], dtype="category") + expected_dtype = np.unicode + + actual_dtype = get_dtype_of_array(array) + + self.assertEqual(expected_dtype, actual_dtype) + + def test__get_dtype_of_array__unordered_integer_categories_return_as_expected(self): + array = Series(data=[2, 3, 1, 3, 1, 2], dtype="category") + expected_dtype = np.int32 + + actual_dtype = get_dtype_of_array(array) + + self.assertEqual(expected_dtype, actual_dtype) + + def test__get_dtype_of_array__castable_dtypes_return_as_expected(self): + types = [np.float64, np.int64] + expected_dtypes = [np.float32, np.int32] + + for test_type_index in range(len(types)): + with self.subTest(f"Testing get_dtype_of_array with castable type {types[test_type_index].__name__}", + i=test_type_index): + array = Series(data=[], dtype=types[test_type_index]) + self.assertEqual(get_dtype_of_array(array), expected_dtypes[test_type_index]) + + def test__get_dtype_of_array__unsupported_type_raises_exception(self): + unsupported_array = Series(list([time() for _ in range(2)]), dtype="datetime64[ns]") + + with self.assertRaises(TypeError) as exception_context: + get_dtype_of_array(unsupported_array) + + self.assertIn("unsupported", str(exception_context.exception)) + def test__get_schema_type_hint_of_array__supported_dtypes_return_as_expected(self): types = [np.float32, np.int32, np.bool_, str] expected_schema_hints = [{"type": "float32"}, {"type": "int32"}, {"type": "boolean"}, {"type": "string"}] @@ -83,14 +146,6 @@ class TestTypeConversionUtils(unittest.TestCase): array = Series(data=[], dtype=types[test_type_index]) self.assertEqual(get_schema_type_hint_of_array(array), expected_schema_hints[test_type_index]) - def test__get_dtype_of_array__categories_return_as_expected(self): - array = Series(data=["a", "b", "c"], dtype="category") - expected_dtype = np.unicode - - actual_dtype = get_dtype_of_array(array) - - self.assertEqual(expected_dtype, actual_dtype) - def test__get_schema_type_hint_of_array__categories_return_as_expected(self): array = Series(data=["a", "b", "b"], dtype="category") expected_schema_hint = {"type": "categorical", "categories": ["a", "b"]} @@ -99,16 +154,6 @@ class TestTypeConversionUtils(unittest.TestCase): self.assertEqual(expected_schema_hint, actual_schema_hint) - def test__get_dtype_of_array__castable_dtypes_return_as_expected(self): - types = [np.float64, np.int64] - expected_dtypes = [np.float32, np.int32] - - for test_type_index in range(len(types)): - with self.subTest(f"Testing get_dtype_of_array with castable type {types[test_type_index].__name__}", - i=test_type_index): - array = Series(data=[], dtype=types[test_type_index]) - self.assertEqual(get_dtype_of_array(array), expected_dtypes[test_type_index]) - def test__get_schema_type_hint_of_array__castable_dtypes_return_as_expected(self): types = [np.float64, np.int64] expected_schema_hints = [{"type": "float32"}, {"type": "int32"}] @@ -133,3 +178,28 @@ class TestTypeConversionUtils(unittest.TestCase): self.assertEqual(expected_data_types_dict, actual_dataframe_data_types) self.assertEqual(expected_schema_type_hints_dict, actual_dataframe_schema_type_hints) + + def test__convert_pandas_series_to_numpy__categorical_float64_to_float64_with_nans(self): + expected_float_array = np.array([1.1, 2.2, np.NaN], dtype=np.float64) + float_series = Series(data=[1.1, 2.2, np.NaN], dtype="category") + + actual_float_array = convert_pandas_series_to_numpy(float_series, np.float64) + + np.testing.assert_equal(expected_float_array, actual_float_array) + + def test__convert_pandas_series_to_numpy__float64_to_float64(self): + expected_float_array = np.array([1.1, 2.2], dtype=np.float64) + float_series = Series(data=[1.1, 2.2], dtype=np.dtype(np.float64)) + + actual_float_array = convert_pandas_series_to_numpy(float_series, np.float64) + + np.testing.assert_equal(expected_float_array, actual_float_array) + + def test__convert_pandas_series_to_numpy__int64_to_int32_with_nans_throws_error(self): + int_series = Series(data=[1, 2, np.NaN], dtype="category") + + with self.assertLogs(level="ERROR") as logger: + convert_pandas_series_to_numpy(int_series, np.int32) + + self.assertIn("Cannot convert a pandas Series object to an integer dtype if it contains NaNs", + logger.output[0]) From 5dfe0043c3bd0562d14c064b5315829fb42b89d1 Mon Sep 17 00:00:00 2001 From: maniarathi Date: Sat, 22 Aug 2020 10:04:40 -0700 Subject: [PATCH 11/15] Serves static assets from each dataset root URL and switch the publicPath to be a relative path. (#1786) --- .../webpack/webpack.config.shared.js | 2 +- client/src/components/menubar/authButtons.js | 2 +- server/app/app.py | 17 +++++++++++++++-- 3 files changed, 17 insertions(+), 4 deletions(-) diff --git a/client/configuration/webpack/webpack.config.shared.js b/client/configuration/webpack/webpack.config.shared.js index 2a8fcfdd..649844a9 100644 --- a/client/configuration/webpack/webpack.config.shared.js +++ b/client/configuration/webpack/webpack.config.shared.js @@ -7,7 +7,7 @@ const ScriptExtHtmlWebpackPlugin = require("script-ext-html-webpack-plugin"); const src = path.resolve("src"); const nodeModules = path.resolve("node_modules"); -const publicPath = "/"; +const publicPath = ""; const rawObsoleteHTMLTemplate = fs.readFileSync( `${__dirname}/obsoleteHTMLTemplate.html`, diff --git a/client/src/components/menubar/authButtons.js b/client/src/components/menubar/authButtons.js index 9323d1e3..7eb4c4c0 100644 --- a/client/src/components/menubar/authButtons.js +++ b/client/src/components/menubar/authButtons.js @@ -19,7 +19,7 @@ const Auth = React.memo((props) => { type="button" data-testid="auth-button" disabled={false} - icon={!userinfo["is_authenticated"] ? "log-in" : "log-out"} + icon={!userinfo.is_authenticated ? "log-in" : "log-out"} href={!userinfo.is_authenticated ? auth.login : auth.logout} > {!userinfo.is_authenticated ? "Log In" : "Log Out"} diff --git a/server/app/app.py b/server/app/app.py index 06612a1c..c84187df 100644 --- a/server/app/app.py +++ b/server/app/app.py @@ -3,7 +3,8 @@ import logging from functools import wraps from http import HTTPStatus -from flask import Flask, redirect, current_app, make_response, render_template, abort, Blueprint, request +from flask import Flask, redirect, current_app, make_response, render_template, abort, Blueprint, request, \ + send_from_directory from flask_restful import Api, Resource from server_timing import Timing as ServerTiming @@ -335,7 +336,7 @@ class Server: pass def __init__(self, app_config): - self.app = Flask(__name__, static_folder="../common/web/static") + self.app = Flask(__name__, static_folder=None) self._before_adding_routes(self.app, app_config) self.app.json_encoder = Float32JSONEncoder server_config = app_config.server_config @@ -369,11 +370,23 @@ class Server: lambda dataset, url_dataroot=url_dataroot: dataset_index(url_dataroot, dataset), methods=["GET"], ) + self.app.add_url_rule( + f"/{url_dataroot}//static/", + f"static_assets_{url_dataroot}", + view_func=lambda dataset, filename: send_from_directory("../common/web/static", filename), + methods=["GET"] + ) else: bp_api = Blueprint("api", __name__, url_prefix=api_version) resources = get_api_resources(bp_api) self.app.register_blueprint(resources.blueprint) + self.app.add_url_rule( + "/static/", + "static_assets", + view_func=lambda filename: send_from_directory("../common/web/static", filename), + methods=["GET"] + ) self.app.matrix_data_cache_manager = server_config.matrix_data_cache_manager self.app.app_config = app_config From 65ea1b673f1b0e3e4f2ca4854a39d74eb229c3f5 Mon Sep 17 00:00:00 2001 From: Madison Dunitz Date: Mon, 24 Aug 2020 18:26:08 -0500 Subject: [PATCH 12/15] 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)) From 9a40b28172252806a3271ebb5ed3478c0734e5dd Mon Sep 17 00:00:00 2001 From: Timmy Huang Date: Mon, 24 Aug 2020 17:05:46 -0700 Subject: [PATCH 13/15] thuang-fix-static-asset-font (#1791) This seems to fix the font URL path, at least locally for both `:3000` and `:5005` Screen Shot 2020-08-24 at 4 01 07 PM Screen Shot 2020-08-24 at 4 01 28 PM --- client/configuration/eslint/eslint.js | 6 +++++ .../webpack/webpack.config.dev.js | 6 ++++- .../webpack/webpack.config.prod.js | 6 ++++- client/server/development.js | 25 +++++++++---------- 4 files changed, 28 insertions(+), 15 deletions(-) diff --git a/client/configuration/eslint/eslint.js b/client/configuration/eslint/eslint.js index 66637c26..21fbfe8d 100644 --- a/client/configuration/eslint/eslint.js +++ b/client/configuration/eslint/eslint.js @@ -64,6 +64,12 @@ module.exports = { "LabeledStatement", "WithStatement", ], + "import/no-extraneous-dependencies": [ + "error", + { + devDependencies: true, + }, + ], }, overrides: [ { diff --git a/client/configuration/webpack/webpack.config.dev.js b/client/configuration/webpack/webpack.config.dev.js index e0881c0a..1295d6dc 100644 --- a/client/configuration/webpack/webpack.config.dev.js +++ b/client/configuration/webpack/webpack.config.dev.js @@ -31,7 +31,11 @@ const devConfig = { test: /\.(jpg|png|gif|eot|svg|ttf|woff|woff2|otf)$/i, loader: "file-loader", include: [nodeModules, fonts], - query: { name: "static/assets/[name].[ext]" }, + query: { + name: "static/assets/[name].[ext]", + // (thuang): This is needed to make sure @font url path is '/static/assets/' + publicPath: "/", + }, }, ], }, diff --git a/client/configuration/webpack/webpack.config.prod.js b/client/configuration/webpack/webpack.config.prod.js index a2115e7f..c92e307f 100644 --- a/client/configuration/webpack/webpack.config.prod.js +++ b/client/configuration/webpack/webpack.config.prod.js @@ -45,7 +45,11 @@ const prodConfig = { test: /\.(jpg|png|gif|eot|svg|ttf|woff|woff2|otf)$/i, loader: "file-loader", include: [nodeModules, fonts], - query: { name: "static/assets/[name]-[contenthash].[ext]" }, + query: { + name: "static/assets/[name]-[contenthash].[ext]", + // (thuang): This is needed to make sure @font url path is '../static/assets/' + publicPath: "static/", + }, }, ], }, diff --git a/client/server/development.js b/client/server/development.js index c85b51ff..0b3e3811 100644 --- a/client/server/development.js +++ b/client/server/development.js @@ -1,20 +1,19 @@ -/* eslint-disable */ -// jshint esversion: 6 -var path = require("path"); -var historyApiFallback = require("connect-history-api-fallback"); -var chalk = require("chalk"); -var express = require("express"); -var favicon = require("serve-favicon"); -var webpack = require("webpack"); -var config = require("../configuration/webpack/webpack.config.dev"); -var utils = require("./utils"); +const path = require("path"); +const historyApiFallback = require("connect-history-api-fallback"); +const chalk = require("chalk"); +const express = require("express"); +const favicon = require("serve-favicon"); +const webpack = require("webpack"); +const devMiddleware = require("webpack-dev-middleware"); +const config = require("../configuration/webpack/webpack.config.dev"); +const utils = require("./utils"); process.env.NODE_ENV = "development"; const CLIENT_PORT = process.env.CXG_CLIENT_PORT; // Set up compiler -var compiler = webpack(config); +const compiler = webpack(config); compiler.plugin("invalid", () => { utils.clearConsole(); @@ -26,12 +25,12 @@ compiler.plugin("done", (stats) => { }); // Launch server -var app = express(); +const app = express(); app.use(historyApiFallback({ verbose: false })); app.use( - require("webpack-dev-middleware")(compiler, { + devMiddleware(compiler, { logLevel: "warn", publicPath: config.output.publicPath, }) From eb05d1cb5c01ea0f6edfbb8e26c6aa8ae4a720a6 Mon Sep 17 00:00:00 2001 From: Prete Date: Tue, 25 Aug 2020 17:37:38 +0100 Subject: [PATCH 14/15] Update Dockerfile (#1775) * Update Dockerfile - Update Ubuntu Focal (20.04) - Add `DEBIAN_FRONTEND=noninteractive` to prevent dialog boxes during installation * Changed 'pip3 install --upgrade pip' to 'python3 -m pip install --upgrade pip' as described here https://github.com/pypa/pip/issues/5599 --- Dockerfile | 1 + 1 file changed, 1 insertion(+) diff --git a/Dockerfile b/Dockerfile index dd695256..cf6389fa 100644 --- a/Dockerfile +++ b/Dockerfile @@ -5,6 +5,7 @@ ENV LANG=C.UTF-8 RUN apt-get update && \ apt-get install -y build-essential libxml2-dev python3-dev python3-pip zlib1g-dev python3-requests python3-aiohttp && \ + python3 -m pip install --upgrade pip && \ pip3 install cellxgene ENTRYPOINT ["cellxgene"] From 0a10b3ec2ae1d3b0f975e6f54cd906d9a495eaff Mon Sep 17 00:00:00 2001 From: Severiano Badajoz Date: Tue, 25 Aug 2020 12:25:55 -0700 Subject: [PATCH 15/15] sort object keys to our specification before generating user colormap (#1792) --- client/src/util/stateManager/colorHelpers.js | 29 ++++++++++++++------ 1 file changed, 20 insertions(+), 9 deletions(-) diff --git a/client/src/util/stateManager/colorHelpers.js b/client/src/util/stateManager/colorHelpers.js index be5fe113..4eaea2b7 100644 --- a/client/src/util/stateManager/colorHelpers.js +++ b/client/src/util/stateManager/colorHelpers.js @@ -94,15 +94,26 @@ export const createColorTable = memoize(_createColorTable); export function loadUserColorConfig(userColors) { const convertedUserColors = {}; Object.keys(userColors).forEach((category) => { - const [colors, scaleMap] = Object.keys(userColors[category]).reduce( - (acc, label, i) => { - const color = parseRGB(userColors[category][label]); - acc[0][label] = color; - acc[1][i] = d3.rgb(255 * color[0], 255 * color[1], 255 * color[2]); - return acc; - }, - [{}, {}] - ); + // We cannot iterate over keys without sorting + // because we handle categorical values in alphabetical order __ignoring case__ + // while Object.keys() _usually_ is ordered alphabetically where all upper characters are less than lowercase (A, B, C, a, b, c) + const [colors, scaleMap] = Object.keys(userColors[category]) + .sort((a, b) => { + a = a.toLowerCase(); + b = b.toLowerCase(); + if (a === b) return 0; + if (a > b) return 1; + return -1; + }) + .reduce( + (acc, label, i) => { + const color = parseRGB(userColors[category][label]); + acc[0][label] = color; + acc[1][i] = d3.rgb(255 * color[0], 255 * color[1], 255 * color[2]); + return acc; + }, + [{}, {}] + ); const scale = (i) => scaleMap[i]; convertedUserColors[category] = { colors, scale }; });