mirror of
https://github.com/chanzuckerberg/cellxgene.git
synced 2026-09-25 07:28:11 +08:00
Makefile modularity, test targets, and auto-formatting (#1070)
* Fix Makefile whitespace and .PHONY use
* Fix Makefile filename
* Modularize Makefile into client and server Makefiles
Part of the reason that the Makefile in the root directory is a bit
complicated is that it tries to handle tasks that can be handled
separately in the client and server modules.
This commit pushes some of the make logic specific to each module into
their own makefiles and calls out to those makefiles from that in the
project root.
* Add auto-formatting to client and server modules
One thing that can make linting faster is auto-formatting. This commit
adds the yapf auto-formatting tool to the server module and uses
eslint's "fix" functionality to speed up the linting/formatting process.
* Add yapf for automatic code formatting
* Add a root test target that calls sub-tests
* Apply yapf to python files
* Do not duplicate npm commands, simply pass through
* Update documentation
* Do not shadow reserved word len
* Add general test target
* Fix make call in dev-env
* Use black instead of yapf
* Run flake8 from the root directory
* Revert "Apply yapf to python files"
This reverts commit cdca128a01.
* Apply black to python code
* Resolve lint errors resulting from black format
* Add explanation of server unit tests in dev guidelines
This commit is contained in:
+98
-66
@@ -25,16 +25,12 @@ def common_args(func):
|
||||
Decorator to contain CLI args that will be common to both CLI and GUI: title and engine args.
|
||||
"""
|
||||
|
||||
@click.option(
|
||||
"--title",
|
||||
"-t",
|
||||
metavar="<text>",
|
||||
help="Title to display. If omitted will use file name.")
|
||||
@click.option("--title", "-t", metavar="<text>", help="Title to display. If omitted will use file name.")
|
||||
@click.option(
|
||||
"--about",
|
||||
metavar="<URL>",
|
||||
help="URL providing more information about the dataset "
|
||||
"(hint: must be a fully specified absolute URL).")
|
||||
help="URL providing more information about the dataset " "(hint: must be a fully specified absolute URL).",
|
||||
)
|
||||
@click.option(
|
||||
"--embedding",
|
||||
"-e",
|
||||
@@ -42,39 +38,43 @@ def common_args(func):
|
||||
multiple=True,
|
||||
show_default=False,
|
||||
metavar="<text>",
|
||||
help="Embedding name, eg, 'umap'. Repeat option for multiple embeddings. Defaults to all."
|
||||
help="Embedding name, eg, 'umap'. Repeat option for multiple embeddings. Defaults to all.",
|
||||
)
|
||||
@click.option(
|
||||
"--obs-names",
|
||||
"-obs",
|
||||
default=None,
|
||||
metavar="<text>",
|
||||
help="Name of annotation field to use for observations. If not specified cellxgene will use the the obs index.")
|
||||
help="Name of annotation field to use for observations. If not specified cellxgene will use the the obs index.",
|
||||
)
|
||||
@click.option(
|
||||
"--var-names",
|
||||
"-var",
|
||||
default=None,
|
||||
metavar="<text>",
|
||||
help="Name of annotation to use for variables. If not specified cellxgene will use the the var index.")
|
||||
help="Name of annotation to use for variables. If not specified cellxgene will use the the var index.",
|
||||
)
|
||||
@click.option(
|
||||
"--max-category-items",
|
||||
default=1000,
|
||||
metavar="<integer>",
|
||||
show_default=True,
|
||||
help="Will not display categories with more distinct values than specified.",)
|
||||
help="Will not display categories with more distinct values than specified.",
|
||||
)
|
||||
@click.option(
|
||||
"--diffexp-lfc-cutoff",
|
||||
"-de",
|
||||
default=0.01,
|
||||
show_default=True,
|
||||
metavar="<float>",
|
||||
help="Minimum log fold change threshold for differential expression.",)
|
||||
help="Minimum log fold change threshold for differential expression.",
|
||||
)
|
||||
@click.option(
|
||||
"--experimental-annotations",
|
||||
is_flag=True,
|
||||
default=False,
|
||||
show_default=True,
|
||||
help="Enable user annotation of data."
|
||||
help="Enable user annotation of data.",
|
||||
)
|
||||
@click.option(
|
||||
"--experimental-annotations-file",
|
||||
@@ -83,7 +83,8 @@ def common_args(func):
|
||||
multiple=False,
|
||||
metavar="<path>",
|
||||
help="CSV file to initialize editing of existing annotations; will be altered in-place. "
|
||||
"Incompatible with --annotations-output-dir.",)
|
||||
"Incompatible with --annotations-output-dir.",
|
||||
)
|
||||
@click.option(
|
||||
"--experimental-annotations-output-dir",
|
||||
default=None,
|
||||
@@ -91,20 +92,23 @@ def common_args(func):
|
||||
multiple=False,
|
||||
metavar="<directory path>",
|
||||
help="Directory of where to save output annotations; filename will be specified in the application. "
|
||||
"Incompatible with --annotations-input-file.",)
|
||||
"Incompatible with --annotations-input-file.",
|
||||
)
|
||||
@click.option(
|
||||
"--backed",
|
||||
"-b",
|
||||
is_flag=True,
|
||||
default=False,
|
||||
show_default=False,
|
||||
help="Load data in file-backed mode. This may save memory, but may result in slower overall performance.")
|
||||
help="Load data in file-backed mode. This may save memory, but may result in slower overall performance.",
|
||||
)
|
||||
@click.option(
|
||||
"--disable-diffexp",
|
||||
is_flag=True,
|
||||
default=False,
|
||||
show_default=False,
|
||||
help="Disable on-demand differential expression.")
|
||||
help="Disable on-demand differential expression.",
|
||||
)
|
||||
@functools.wraps(func)
|
||||
def wrapper(*args, **kwargs):
|
||||
return func(*args, **kwargs)
|
||||
@@ -112,9 +116,18 @@ def common_args(func):
|
||||
return wrapper
|
||||
|
||||
|
||||
def parse_engine_args(embedding, obs_names, var_names, max_category_items, diffexp_lfc_cutoff,
|
||||
experimental_annotations, experimental_annotations_file,
|
||||
experimental_annotations_output_dir, backed, disable_diffexp):
|
||||
def parse_engine_args(
|
||||
embedding,
|
||||
obs_names,
|
||||
var_names,
|
||||
max_category_items,
|
||||
diffexp_lfc_cutoff,
|
||||
experimental_annotations,
|
||||
experimental_annotations_file,
|
||||
experimental_annotations_output_dir,
|
||||
backed,
|
||||
disable_diffexp,
|
||||
):
|
||||
annotations_file = experimental_annotations_file if experimental_annotations else None
|
||||
annotations_output_dir = experimental_annotations_output_dir if experimental_annotations else None
|
||||
return {
|
||||
@@ -127,14 +140,15 @@ def parse_engine_args(embedding, obs_names, var_names, max_category_items, diffe
|
||||
"annotations_file": annotations_file,
|
||||
"annotations_output_dir": annotations_output_dir,
|
||||
"backed": backed,
|
||||
"disable_diffexp": disable_diffexp
|
||||
"disable_diffexp": disable_diffexp,
|
||||
}
|
||||
|
||||
|
||||
@sort_options
|
||||
@click.command(short_help="Launch the cellxgene data viewer. "
|
||||
"Run `cellxgene launch --help` for more information.",
|
||||
options_metavar="<options>",)
|
||||
@click.command(
|
||||
short_help="Launch the cellxgene data viewer. " "Run `cellxgene launch --help` for more information.",
|
||||
options_metavar="<options>",
|
||||
)
|
||||
@click.argument("data", nargs=1, metavar="<path to data file>", required=True)
|
||||
@click.option(
|
||||
"--verbose",
|
||||
@@ -142,7 +156,8 @@ def parse_engine_args(embedding, obs_names, var_names, max_category_items, diffe
|
||||
is_flag=True,
|
||||
default=False,
|
||||
show_default=True,
|
||||
help="Provide verbose output, including warnings and all server requests.",)
|
||||
help="Provide verbose output, including warnings and all server requests.",
|
||||
)
|
||||
@click.option(
|
||||
"--debug",
|
||||
"-d",
|
||||
@@ -150,7 +165,8 @@ def parse_engine_args(embedding, obs_names, var_names, max_category_items, diffe
|
||||
default=False,
|
||||
show_default=True,
|
||||
help="Run in debug mode. This is helpful for cellxgene developers, "
|
||||
"or when you want more information about an error condition.",)
|
||||
"or when you want more information about an error condition.",
|
||||
)
|
||||
@click.option(
|
||||
"--open",
|
||||
"-o",
|
||||
@@ -158,19 +174,22 @@ def parse_engine_args(embedding, obs_names, var_names, max_category_items, diffe
|
||||
is_flag=True,
|
||||
default=False,
|
||||
show_default=True,
|
||||
help="Open web browser after launch.",)
|
||||
help="Open web browser after launch.",
|
||||
)
|
||||
@click.option(
|
||||
"--port",
|
||||
"-p",
|
||||
metavar="<port>",
|
||||
show_default=True,
|
||||
help="Port to run server on. If not specified cellxgene will find an available port.",)
|
||||
help="Port to run server on. If not specified cellxgene will find an available port.",
|
||||
)
|
||||
@click.option(
|
||||
"--host",
|
||||
metavar="<IP address>",
|
||||
default="127.0.0.1",
|
||||
show_default=False,
|
||||
help="Host IP address. By default cellxgene will use localhost (e.g. 127.0.0.1).")
|
||||
help="Host IP address. By default cellxgene will use localhost (e.g. 127.0.0.1).",
|
||||
)
|
||||
@click.option(
|
||||
"--scripts",
|
||||
"-s",
|
||||
@@ -178,30 +197,31 @@ def parse_engine_args(embedding, obs_names, var_names, max_category_items, diffe
|
||||
multiple=True,
|
||||
metavar="<text>",
|
||||
help="Additional script files to include in HTML page. If not specified, "
|
||||
"no additional script files will be included.",
|
||||
show_default=False,)
|
||||
"no additional script files will be included.",
|
||||
show_default=False,
|
||||
)
|
||||
@click.help_option("--help", "-h", help="Show this message and exit.")
|
||||
@common_args
|
||||
def launch(
|
||||
data,
|
||||
verbose,
|
||||
debug,
|
||||
open_browser,
|
||||
port,
|
||||
host,
|
||||
embedding,
|
||||
obs_names,
|
||||
var_names,
|
||||
max_category_items,
|
||||
diffexp_lfc_cutoff,
|
||||
title,
|
||||
scripts,
|
||||
about,
|
||||
experimental_annotations,
|
||||
experimental_annotations_file,
|
||||
experimental_annotations_output_dir,
|
||||
backed,
|
||||
disable_diffexp
|
||||
data,
|
||||
verbose,
|
||||
debug,
|
||||
open_browser,
|
||||
port,
|
||||
host,
|
||||
embedding,
|
||||
obs_names,
|
||||
var_names,
|
||||
max_category_items,
|
||||
diffexp_lfc_cutoff,
|
||||
title,
|
||||
scripts,
|
||||
about,
|
||||
experimental_annotations,
|
||||
experimental_annotations_file,
|
||||
experimental_annotations_output_dir,
|
||||
backed,
|
||||
disable_diffexp,
|
||||
):
|
||||
"""Launch the cellxgene data viewer.
|
||||
This web app lets you explore single-cell expression data.
|
||||
@@ -217,13 +237,18 @@ def launch(
|
||||
|
||||
> cellxgene launch <url>"""
|
||||
|
||||
e_args = parse_engine_args(embedding, obs_names, var_names, max_category_items,
|
||||
diffexp_lfc_cutoff,
|
||||
experimental_annotations,
|
||||
experimental_annotations_file,
|
||||
experimental_annotations_output_dir,
|
||||
backed,
|
||||
disable_diffexp)
|
||||
e_args = parse_engine_args(
|
||||
embedding,
|
||||
obs_names,
|
||||
var_names,
|
||||
max_category_items,
|
||||
diffexp_lfc_cutoff,
|
||||
experimental_annotations,
|
||||
experimental_annotations_file,
|
||||
experimental_annotations_output_dir,
|
||||
backed,
|
||||
disable_diffexp,
|
||||
)
|
||||
try:
|
||||
data_locator = DataLocator(data)
|
||||
except RuntimeError as re:
|
||||
@@ -256,7 +281,8 @@ def launch(
|
||||
sys.tracebacklimit = 0
|
||||
|
||||
if scripts:
|
||||
click.echo(r"""
|
||||
click.echo(
|
||||
r"""
|
||||
/ / /\ \ \__ _ _ __ _ __ (_)_ __ __ _
|
||||
\ \/ \/ / _` | '__| '_ \| | '_ \ / _` |
|
||||
\ /\ / (_| | | | | | | | | | | (_| |
|
||||
@@ -264,7 +290,8 @@ def launch(
|
||||
|___/
|
||||
The --scripts flag is intended for developers to include google analytics etc. You could be opening yourself to a
|
||||
security risk by including the --scripts flag. Make sure you trust the scripts that you are including.
|
||||
""")
|
||||
"""
|
||||
)
|
||||
scripts_pretty = ", ".join(scripts)
|
||||
click.confirm(f"Are you sure you want to inject these scripts: {scripts_pretty}?", abort=True)
|
||||
|
||||
@@ -289,8 +316,9 @@ def launch(
|
||||
click.echo("Warning: --experimental-annotations-output-dir ignored as --annotations not enabled.")
|
||||
else:
|
||||
if experimental_annotations_file is not None and experimental_annotations_output_dir is not None:
|
||||
raise click.ClickException("--experimental-annotations-file and --experimental-annotations-output-dir "
|
||||
"may not be used together.")
|
||||
raise click.ClickException(
|
||||
"--experimental-annotations-file and --experimental-annotations-output-dir " "may not be used together."
|
||||
)
|
||||
|
||||
if experimental_annotations_file is not None:
|
||||
lf_name, lf_ext = splitext(experimental_annotations_file)
|
||||
@@ -301,10 +329,12 @@ def launch(
|
||||
try:
|
||||
mkdir(experimental_annotations_output_dir)
|
||||
except OSError:
|
||||
raise click.ClickException("Unable to create directory specified by "
|
||||
"--experimental-annotations-output-dir")
|
||||
raise click.ClickException(
|
||||
"Unable to create directory specified by " "--experimental-annotations-output-dir"
|
||||
)
|
||||
|
||||
if about:
|
||||
|
||||
def url_check(url):
|
||||
try:
|
||||
result = urlparse(url)
|
||||
@@ -346,9 +376,11 @@ def launch(
|
||||
except ScanpyFileError as e:
|
||||
raise click.ClickException(f"{e}")
|
||||
|
||||
if not disable_diffexp and server.app.data.config['diffexp_may_be_slow']:
|
||||
click.echo(f"[cellxgene] CAUTION: due to the size of your dataset, "
|
||||
f"running differential expression may take longer or fail.")
|
||||
if not disable_diffexp and server.app.data.config["diffexp_may_be_slow"]:
|
||||
click.echo(
|
||||
f"[cellxgene] CAUTION: due to the size of your dataset, "
|
||||
f"running differential expression may take longer or fail."
|
||||
)
|
||||
|
||||
if open_browser:
|
||||
click.echo(f"[cellxgene] Launching! Opening your browser to {cellxgene_url} now.")
|
||||
|
||||
Reference in New Issue
Block a user