Repository: dandi/dandi-cli

Issue:
ls: alternative output formats
we would like to be able to spit out metadata in alternative formats/renderings
- json
- json_pp (pretty printed) (both might be not a single json record, but rather [json lines](http://jsonlines.org/) stream)
- yaml (may be when/if interest/demand)
- pandas dataframe rendering (will require obtaining all records first)

Discussion:

Patch:
diff --git a/.gitignore b/.gitignore
index fdd0b550..608569e1 100644
--- a/.gitignore
+++ b/.gitignore
@@ -6,3 +6,4 @@ dist/
 pip-wheel-metadata/
 sandbox/
 venvs/
+.coverage
diff --git a/dandi/__init__.py b/dandi/__init__.py
index 0a9cad5d..a6caf46a 100644
--- a/dandi/__init__.py
+++ b/dandi/__init__.py
@@ -1,5 +1,5 @@
 import logging
-
+import os
 from . import _version
 
 __version__ = _version.get_versions()["version"]
@@ -16,10 +16,23 @@ def get_logger(name=None):
     return logging.getLogger("dandi" + (".%s" % name if name else ""))
 
 
+def set_logger_level(lgr, level):
+    if isinstance(level, int):
+        pass
+    elif level.isnumeric():
+        level = int(level)
+    elif level.isalpha():
+        level = getattr(logging, level)
+    else:
+        lgr.warning("Do not know how to treat loglevel %s" % level)
+        return
+    lgr.setLevel(level)
+
+
 _DEFAULT_LOG_FORMAT = "%(asctime)s - %(name)s - %(levelname)s - %(message)s"
 
 lgr = get_logger()
 # Basic settings for output, for now just basic
-lgr.setLevel(logging.INFO)
+set_logger_level(lgr, os.environ.get("DANDI_LOG_LEVEL", logging.INFO))
 FORMAT = "%(asctime)-15s [%(levelname)8s] %(message)s"
 logging.basicConfig(format=FORMAT)
diff --git a/dandi/cli/command.py b/dandi/cli/command.py
index add8f750..e9948797 100644
--- a/dandi/cli/command.py
+++ b/dandi/cli/command.py
@@ -9,10 +9,8 @@ import sys
 import click
 from click_didyoumean import DYMGroup
 
-import pyout
-
 import logging
-from .. import get_logger
+from .. import get_logger, set_logger_level
 
 from collections import OrderedDict
 
@@ -21,7 +19,6 @@ from .. import __version__
 # Delay imports leading to import of heavy modules such as pynwb and h5py
 # Import at the point of use
 # from ..pynwb_utils import ...
-from ..support import pyout as pyouts
 
 lgr = get_logger()
 
@@ -51,7 +48,7 @@ def print_version(ctx, param, value):
 )
 @click.option("--pdb", help="Fall into pdb if errors out", is_flag=True)
 def main(log_level, pdb=False):
-    get_logger().setLevel(log_level)  # common one
+    set_logger_level(get_logger(), log_level)  # common one
     if pdb:
         from ..utils import setup_exceptionhook
 
@@ -76,23 +73,25 @@ def get_files(paths, recursive=True, recurion_limit=None):
     return paths
 
 
-def get_metadata_pyout(path, keys):
+def get_metadata_pyout(path, keys=None):
     from ..pynwb_utils import get_metadata, get_nwb_version
 
     def fn():
         rec = {}
-        try:
-            meta = get_metadata(path)
-            # normalize some fields and remove completely empty
-            for f, v in meta.items():
-                if f not in keys:
-                    continue
-                if isinstance(v, (tuple, list)):
-                    v = ", ".join(v)
-                if v:
-                    rec[f] = v
-        except Exception as exc:
-            lgr.debug("Failed to get metadata from %s: %s", path, exc)
+        # No need for calling get_metadata if no keys are needed from it
+        if keys is None or list(keys) != ["nwb_version"]:
+            try:
+                meta = get_metadata(path)
+                # normalize some fields and remove completely empty
+                for f, v in meta.items():
+                    if keys is not None and f not in keys:
+                        continue
+                    if isinstance(v, (tuple, list)):
+                        v = ", ".join(v)
+                    if v:
+                        rec[f] = v
+            except Exception as exc:
+                lgr.debug("Failed to get metadata from %s: %s", path, exc)
 
         if "nwb_version" not in rec:
             # Let's at least get that one
@@ -107,127 +106,102 @@ def get_metadata_pyout(path, keys):
 
 
 @main.command()
+@click.option(
+    "-F",
+    "--fields",
+    help="Comma-separated list of fields to display. "
+    "An empty value to trigger a list of "
+    "available fields to be printed out",
+)
+@click.option(
+    "-f",
+    "--format",
+    help="Choose the format/frontend for output. If 'auto', 'pyout' will be "
+    "used in case of multiple files, and 'yaml' for a single file.",
+    type=click.Choice(["auto", "pyout", "json", "json_pp", "yaml"]),
+    default="auto",
+)
 @click.argument("paths", nargs=-1, type=click.Path(exists=True, dir_okay=False))
-def ls(paths):
+def ls(paths, fields=None, format="auto"):
     """List file size and selected set of metadata fields
     """
+    from ..consts import metadata_all_fields
+
+    all_fields = sorted(["path", "size"] + list(metadata_all_fields))
+
+    # TODO: more logical ordering in case of fields = None
+    from .formatter import JSONFormatter, YAMLFormatter, PYOUTFormatter
+
+    # TODO: avoid
+    from .formatter import PYOUT_SHORT_NAMES, PYOUT_SHORT_NAMES_rev
+
+    if fields is not None:
+        if fields.strip() == "":
+            for field in all_fields:
+                s = field
+                if field in PYOUT_SHORT_NAMES:
+                    s += " or %s" % PYOUT_SHORT_NAMES[field]
+                click.secho(s)
+            raise SystemExit(0)
+        fields = fields.split(",")
+        # Map possibly present short names back to full names
+        fields = [PYOUT_SHORT_NAMES_rev.get(f.lower(), f) for f in fields]
+        unknown_fields = set(fields).difference(all_fields)
+        if unknown_fields:
+            raise ValueError(
+                "Following fields are not known: %s" % ", ".join(unknown_fields)
+            )
+
     # For now we support only individual files
     files = get_files(paths)
 
     if not files:
         return
 
-    max_filename_len = max(map(lambda x: len(op.basename(x)), files))
-    # Needs to stay here due to use of  counds/mapped_counts
-    PYOUT_STYLE = OrderedDict(
-        [
-            ("summary_", {"bold": True}),
-            (
-                "header_",
-                dict(
-                    bold=True,
-                    transform=lambda x: {
-                        # shortening for some fields
-                        "nwb_version": "NWB",
-                        #'annex_local_size': 'annex(present)',
-                        #'annex_worktree_size': 'annex(worktree)',
-                    }
-                    .get(x, x)
-                    .upper(),
-                ),
-            ),
-            # ('default_', dict(align="center")),
-            ("default_", dict(missing="")),
-            (
-                "path",
-                dict(
-                    bold=True,
-                    align="left",
-                    underline=True,
-                    width=dict(
-                        truncate="left",
-                        # min=max_filename_len + 4 #  .../
-                        # min=0.3  # not supported yet by pyout, https://github.com/pyout/pyout/issues/85
-                    ),
-                    aggregate=lambda _: "Summary:"
-                    # TODO: seems to be wrong
-                    # width='auto'
-                    # summary=lambda x: "TOTAL: %d" % len(x)
-                ),
-            ),
-            # ('type', dict(
-            #     transform=lambda s: "%s" % s,
-            #     aggregate=counts,
-            #     missing='-',
-            #     # summary=summary_counts
-            # )),
-            # ('describe', dict(
-            #     transform=empty_for_none)),
-            # ('clean', dict(
-            #     color='green',
-            #     transform=fancy_bool,
-            #     aggregate=mapped_counts({False: fancy_bool(False),
-            #                              True: fancy_bool(True)}),
-            #     delayed="group-git"
-            # )),
-            ("size", pyouts.size_style),
-            (
-                "session_start_time",
-                dict(
-                    transform=pyouts.datefmt,
-                    aggregate=pyouts.summary_dates,
-                    # summary=summary_dates
-                ),
-            ),
-        ]
-    )
-    if not sys.stdout.isatty():
-        # TODO: ATM width in the final mode is hardcoded
-        #  https://github.com/pyout/pyout/issues/70
-        # and depending on how it would be resolved, there might be a
-        # need to specify it here as "max" or smth like that.
-        # For now hardcoding to hopefully wide enough 200 if stdout is not
-        # a tty
-        PYOUT_STYLE["width_"] = 200
-
-    out = pyout.Tabular(
-        columns=[
-            "path",
-            "nwb_version",
-            "size",
-            #'experiment_description',
-            "lab",
-            "experimenter",
-            "session_id",
-            "subject_id",
-            "session_start_time",
-            #'identifier',  # note: required arg2 of NWBFile
-            #'institution',
-            "keywords",
-            #'related_publications',
-            #'session_description',  # note: required arg1 of NWBFile
-        ],
-        style=PYOUT_STYLE
-        # , stream=...
-    )
+    if format == "auto":
+        format = "yaml" if len(files) == 1 else "pyout"
+
+    if format == "pyout":
+        out = PYOUTFormatter(files=files, fields=fields)
+    elif format == "json":
+        out = JSONFormatter()
+    elif format == "json_pp":
+        out = JSONFormatter(indent=2)
+    elif format == "yaml":
+        out = YAMLFormatter()
+    else:
+        raise NotImplementedError("Unknown format %s" % format)
+
+    if fields is not None:
+        async_keys = tuple(set(metadata_all_fields).intersection(fields))
+    else:
+        async_keys = metadata_all_fields
+
     with out:
-        async_keys = (
-            "NWB",
-            "lab",
-            "experimenter",
-            "session_id",
-            "subject_id",
-            "session_start_time",
-            "keywords",
-        )
         for path in files:
-            rec = {"path": path}
+            rec = {}
+            rec["path"] = path
+
             try:
-                rec["size"] = os.stat(path).st_size
-                rec[async_keys] = get_metadata_pyout(path, async_keys)
+                if not fields or "size" in fields:
+                    rec["size"] = os.stat(path).st_size
+
+                if async_keys:
+                    cb = get_metadata_pyout(path, async_keys)
+                    if format == "pyout":
+                        rec[async_keys] = cb
+                    else:
+                        # TODO: parallel execution
+                        # For now just call callback and get all the fields
+                        for k, v in cb().items():
+                            rec[k] = v
             except FileNotFoundError as exc:
-                # lgr.error("File is not available: %s", exc)
-                pass
+                lgr.debug("File is not available: %s", exc)
+            except Exception as exc:
+                lgr.debug("Problem obtaining metadata for %s: %s", path, exc)
+            if not rec:
+                lgr.debug("Skipping a record for %s since emtpy", path)
+                continue
             out(rec)
 
 
diff --git a/dandi/cli/formatter.py b/dandi/cli/formatter.py
new file mode 100644
index 00000000..f6a2eda5
--- /dev/null
+++ b/dandi/cli/formatter.py
@@ -0,0 +1,140 @@
+import datetime
+import os.path as op
+import sys
+
+import pyout
+from ..support import pyout as pyouts
+
+from .. import get_logger
+
+lgr = get_logger()
+
+
+class Formatter(object):
+    def __enter__(self):
+        pass
+
+    def __exit__(self, exc_type, exc_value, traceback):
+        pass
+
+    def __call__(self, rec):
+        pass
+
+
+class JSONFormatter(Formatter):
+    def __init__(self, indent=None, out=None):
+        self.out = out or sys.stdout
+        self.indent = indent
+
+    @staticmethod
+    def _serializer(o):
+        if isinstance(o, datetime.datetime):
+            return o.__str__()
+        return o
+
+    def __call__(self, rec):
+        import json
+
+        self.out.write(
+            json.dumps(rec, indent=self.indent, default=self._serializer) + "\n"
+        )
+
+
+class YAMLFormatter(Formatter):
+    def __init__(self, out=None):
+        self.out = out or sys.stdout
+        self.records = []
+
+    def __exit__(self, exc_type, exc_value, traceback):
+        import yaml
+
+        self.out.write(yaml.dump(self.records))
+
+    def __call__(self, rec):
+        self.records.append(rec)
+
+
+PYOUT_SHORT_NAMES = {
+    # shortening for some fields
+    "nwb_version": "NWB",
+    "number_of_electrodes": "#electrodes",
+    "number_of_units": "#units",
+    # 'annex_local_size': 'annex(present)',
+    # 'annex_worktree_size': 'annex(worktree)',
+}
+# For reverse lookup
+PYOUT_SHORT_NAMES_rev = {v.lower(): k for k, v in PYOUT_SHORT_NAMES.items()}
+
+
+class PYOUTFormatter(pyout.Tabular):
+    def __init__(self, files, fields):
+        max_filename_len = max(map(lambda x: len(op.basename(x)), files))
+        # Needs to stay here due to use of  counts/mapped_counts
+        PYOUT_STYLE = {
+            "summary_": {"bold": True},
+            "header_": dict(
+                bold=True, transform=lambda x: PYOUT_SHORT_NAMES.get(x, x).upper()
+            ),
+            "default_": dict(missing=""),
+            "path": dict(
+                bold=True,
+                align="left",
+                underline=True,
+                width=dict(
+                    truncate="left",
+                    # min=max_filename_len + 4 #  .../
+                    # min=0.3  # not supported yet by pyout,
+                    # https://github.com/pyout/pyout/issues/85
+                ),
+                aggregate=lambda _: "Summary:"
+                # TODO: seems to be wrong
+                # width='auto'
+                # summary=lambda x: "TOTAL: %d" % len(x)
+            ),
+            # ('type', dict(
+            #     transform=lambda s: "%s" % s,
+            #     aggregate=counts,
+            #     missing='-',
+            #     # summary=summary_counts
+            # )),
+            # ('describe', dict(
+            #     transform=empty_for_none)),
+            # ('clean', dict(
+            #     color='green',
+            #     transform=fancy_bool,
+            #     aggregate=mapped_counts({False: fancy_bool(False),
+            #                              True: fancy_bool(True)}),
+            #     delayed="group-git"
+            # )),
+            "size": dict(pyouts.size_style),
+            "nwb_version": {},  # Just to establish the order
+            "session_start_time": dict(
+                transform=pyouts.datefmt,
+                aggregate=pyouts.summary_dates,
+                # summary=summary_dates
+            ),
+        }
+        # To just quickly switch for testing released or not released (with
+        # hide)
+        # pyout
+        if "hide" in pyout.elements.schema["definitions"]:
+            lgr.debug("pyout with 'hide' support detected")
+            PYOUT_STYLE["default_"]["hide"] = "if_missing" if not fields else False
+            for f in "path", "size":
+                PYOUT_STYLE[f]["hide"] = False
+        else:
+            lgr.warning("pyout without 'hide' support. Expect too many columns")
+
+        if not sys.stdout.isatty():
+            # TODO: ATM width in the final mode is hardcoded
+            #  https://github.com/pyout/pyout/issues/70
+            # and depending on how it would be resolved, there might be a
+            # need to specify it here as "max" or smth like that.
+            # For now hardcoding to hopefully wide enough 200 if stdout is not
+            # a tty
+            PYOUT_STYLE["width_"] = 200
+
+        kw = dict(style=PYOUT_STYLE)
+        if fields:
+            kw["columns"] = fields
+        super().__init__(**kw)
diff --git a/dandi/consts.py b/dandi/consts.py
new file mode 100644
index 00000000..9ba3bddc
--- /dev/null
+++ b/dandi/consts.py
@@ -0,0 +1,30 @@
+# A list of metadata fields which dandi extracts from .nwb files.
+# Additional fields (such as `number_of_*`) might be added by the
+# get_metadata`
+metadata_fields = (
+    "experiment_description",
+    "experimenter",
+    "identifier",  # note: required arg2 of NWBFile
+    "institution",
+    "keywords",
+    "lab",
+    "related_publications",
+    "session_description",  # note: required arg1 of NWBFile
+    "session_id",
+    "session_start_time",
+)
+
+metadata_subject_fields = (
+    "age",
+    "date_of_birth",
+    "genotype",
+    "sex",
+    "species",
+    "subject_id",
+)
+
+metadata_computed_fields = ("number_of_electrodes", "number_of_units", "nwb_version")
+
+metadata_all_fields = (
+    metadata_fields + metadata_subject_fields + metadata_computed_fields
+)
diff --git a/dandi/pynwb_utils.py b/dandi/pynwb_utils.py
index 32e3fc64..39af2830 100644
--- a/dandi/pynwb_utils.py
+++ b/dandi/pynwb_utils.py
@@ -5,30 +5,7 @@ from . import get_logger
 
 lgr = get_logger()
 
-# A list of metadata fields which dandi extracts from .nwb files.
-# Additional fields (such as `number_of_*`) might be added by the
-# get_metadata`
-metadata_fields = (
-    "experiment_description",
-    "experimenter",
-    "identifier",  # note: required arg2 of NWBFile
-    "institution",
-    "keywords",
-    "lab",
-    "related_publications",
-    "session_description",  # note: required arg1 of NWBFile
-    "session_id",
-    "session_start_time",
-)
-
-metadata_subject_fields = (
-    "age",
-    "date_of_birth",
-    "genotype",
-    "sex",
-    "species",
-    "subject_id",
-)
+from .consts import metadata_fields, metadata_computed_fields, metadata_subject_fields
 
 
 def get_nwb_version(filepath):
@@ -85,7 +62,14 @@ def get_metadata(filepath):
         # Add a few additional useful fields
 
         # Counts
-        for f in ["electrodes", "units"]:
-            out["number_of_{}".format(f)] = len(getattr(nwb, f, []) or [])
+        for f in metadata_computed_fields:
+            if f in ("nwb_version",):
+                continue
+            if not f.startswith("number_of_"):
+                raise NotImplementedError(
+                    "ATM can only compute number_of_ fields. Got {}".format(f)
+                )
+            key = f[len("number_of_") :]
+            out[f] = len(getattr(nwb, key, []) or [])
 
     return out
diff --git a/setup.cfg b/setup.cfg
index 686e9742..024f04ef 100644
--- a/setup.cfg
+++ b/setup.cfg
@@ -35,6 +35,7 @@ install_requires =
     click-didyoumean
     pyout
     humanize
+    pyyaml
 tests_require =
     pytest
 # TODO: figure out. For now alias test


Test patch:
diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
index 5cb38de1..0bba5ff9 100644
--- a/.github/workflows/test.yml
+++ b/.github/workflows/test.yml
@@ -12,7 +12,7 @@ jobs:
       fail-fast: false
       matrix:
         os:
-          - windows-2019,
+          - windows-2019
           - ubuntu-18.04
         python:
           - 3.6
@@ -30,7 +30,7 @@ jobs:
     - name: Install dependencies
       run: |
         python -m pip install --upgrade pip
-        pip install ".[tests]"
+        pip install ".[test]"
     - name: ${{ matrix.module }} tests
       run: |
         python -m pytest -s -v --cov=dandi dandi
diff --git a/dandi/cli/tests/test_command.py b/dandi/cli/tests/test_command.py
index bad43ef1..70a5dd8b 100644
--- a/dandi/cli/tests/test_command.py
+++ b/dandi/cli/tests/test_command.py
@@ -10,13 +10,13 @@ import pytest
 @pytest.mark.parametrize("command", (ls, validate))
 def test_smoke(simple1_nwb, command):
     runner = CliRunner()
-    result = runner.invoke(command, [simple1_nwb])
-    assert result.exit_code == 0
-    # we would need to redirect pyout for its analysis
+    r = runner.invoke(command, [simple1_nwb])
+    assert r.exit_code == 0, f"Exited abnormally. out={r.stdout}"
+    assert r.stdout, "There were no output whatsoever"
 
     # empty invocation should not crash
-    result = runner.invoke(command, [])
-    assert result.exit_code == 0
+    r = runner.invoke(command, [])
+    assert r.exit_code == 0, f"Exited abnormally. out={r.stdout}"
 
 
 def test_no_heavy_imports():
diff --git a/dandi/cli/tests/test_ls.py b/dandi/cli/tests/test_ls.py
new file mode 100644
index 00000000..5b7e886a
--- /dev/null
+++ b/dandi/cli/tests/test_ls.py
@@ -0,0 +1,36 @@
+from click.testing import CliRunner
+import pytest
+
+from ..command import ls
+from ...pynwb_utils import metadata_fields
+
+
+@pytest.mark.parametrize("format", ("auto", "json", "json_pp", "yaml", "pyout"))
+def test_smoke(simple1_nwb_metadata, simple1_nwb, format):
+    runner = CliRunner()
+    r = runner.invoke(ls, ["-f", format, simple1_nwb])
+    assert r.exit_code == 0, f"Exited abnormally. out={r.stdout}"
+    # we would need to redirect pyout for its analysis
+    out = r.stdout
+
+    if format.startswith("json"):
+        import json
+
+        load = json.loads
+    elif format == "yaml":
+        import yaml
+
+        def load(s):
+            obj = yaml.load(s, Loader=yaml.BaseLoader)
+            assert len(obj) == 1  # will be a list with a single elem
+            return obj[0]
+
+    else:
+        return
+
+    metadata = load(out)
+    assert metadata
+    # check a few fields
+    assert metadata.pop("nwb_version").startswith("2.")
+    for f in ["session_id", "experiment_description"]:
+        assert metadata[f] == simple1_nwb_metadata[f]