support config files contributions

.github/workflows/tests.yml

@@ -8,17 +8,14 @@ jobs:
     strategy:
       matrix:
         os: [ubuntu-latest, macos-latest, windows-latest]
-        python-version: ['3.6', '3.7', '3.8', '3.9', '3.10']
+        python-version: ['3.7', '3.8', '3.9', '3.10', '3.11']
 
     steps:
-    - uses: actions/checkout@v2
+    - uses: actions/checkout@v3
     - name: Set up Python ${{ matrix.python-version }}
-      uses: actions/setup-python@v2
+      uses: actions/setup-python@v4
       with:
         python-version: ${{ matrix.python-version }}
-    - name: Install Python 3.6 dependencies
-      if: ${{ matrix.python-version == '3.6' }}
-      run: python -m pip install 'traitlets<5' 'nbformat==5.1.3'
     - name: Install dependencies
       run: |
         python -m pip install wheel
@@ -28,10 +25,12 @@ jobs:
       run: git config --global init.defaultBranch main
     - name: Run tests (Linux)
       if: matrix.os == 'ubuntu-latest'
+      shell: bash
       run: |
         pytest
     - name: Run tests (MacOS)
       if: matrix.os == 'macos-latest'
+      shell: bash
       run: |
         # Ignore test-hg.t as Mercurial is not installed
         echo 'cramignore = test-hg.t' >> pytest.ini
@@ -41,10 +40,7 @@ jobs:
       shell: bash
       env:
         NBSTRIPOUT_EXE: ${{ env.pythonLocation }}\Scripts\nbstripout.exe
-        CRAMSHELL: bash
       run: |
         git config --global core.autocrlf true
-        echo NBSTRIPOUT_EXE=${NBSTRIPOUT_EXE}
-        ${NBSTRIPOUT_EXE} --help
         # cram is broken on Windows (#38)
-        # pytest
+        pytest --nocram

CHANGELOG.rst

@@ -98,7 +98,7 @@ Change Log
 0.3.4 - 2019-03-26
 ------------------
 * Fix ``WindowsError`` not defined on POSIX systems (#90)
-* Add support for blacklisting custom metatdata fields (#92, @casperdcl)
+* Add support for blacklisting custom metadata fields (#92, @casperdcl)
 
 0.3.3 - 2018-08-04
 ------------------
@@ -205,7 +205,7 @@ Change Log
 
 0.1.0 - not released
 --------------------
-* Based on Min RK's orginal but supports multiple versions of
+* Based on Min RK's original but supports multiple versions of
   IPython/Jupyter and also strips the execution count.
 * Add install option that fails sensibly if not in a git repository,
   does not clobber an existing attributes file and checks for an

README.rst

@@ -405,7 +405,7 @@ Using ``nbstripout`` as a pre-commit hook
 
 `pre-commit`_ is a framework for managing git `pre-commit hooks`_.
 
-Once you have `pre-commit`_ installed, add the follwong to the
+Once you have `pre-commit`_ installed, add the following to the
 ``.pre-commit-config.yaml`` in your repository: ::
 
     repos:
@@ -438,7 +438,7 @@ Certain Git workflows are not well supported by `nbstripout`:
   `nbstripout` filter do still cause conflicts when attempting to sync upstream
   changes (`git pull`, `git merge` etc.). This is because Git has no way of
   resolving a conflict caused by a non-stripped local file being merged with a
-  stripped upstream file. Adressing this issue is out of scope for `nbstripout`.
+  stripped upstream file. Addressing this issue is out of scope for `nbstripout`.
   Read more and find workarounds in `#108`_.
 
 .. _#108: https://github.com/kynan/nbstripout/issues/108

nbstripout/_nbstripout.py

@@ -109,7 +109,7 @@
     *.ipynb diff=ipynb
 """
 
-from argparse import ArgumentParser, RawDescriptionHelpFormatter
+from argparse import ArgumentParser, Namespace, RawDescriptionHelpFormatter
 import collections
 import io
 import json
@@ -224,10 +224,10 @@ def install(git_config, install_location=INSTALL_LOCATION_LOCAL, attrfile=None):
         attrfile = _get_attrfile(git_config, install_location, attrfile)
     except FileNotFoundError:
         print('Installation failed: git is not on path!', file=sys.stderr)
-        sys.exit(1)
+        return 1
     except CalledProcessError:
         print('Installation failed: not a git repository!', file=sys.stderr)
-        sys.exit(1)
+        return 1
 
     # Check if there is already a filter for ipynb files
     filt_exists = False
@@ -262,7 +262,7 @@ def install(git_config, install_location=INSTALL_LOCATION_LOCAL, attrfile=None):
         if install_location == INSTALL_LOCATION_GLOBAL:
             print('Did you forget to sudo?', file=sys.stderr)
 
-        sys.exit(1)
+        return 1
 
 
 def uninstall(git_config, install_location=INSTALL_LOCATION_LOCAL, attrfile=None):
@@ -274,10 +274,10 @@ def uninstall(git_config, install_location=INSTALL_LOCATION_LOCAL, attrfile=None
         attrfile = _get_attrfile(git_config, install_location, attrfile)
     except FileNotFoundError:
         print('Uninstall failed: git is not on path!', file=sys.stderr)
-        sys.exit(1)
+        return 1
     except CalledProcessError:
         print('Uninstall failed: not a git repository!', file=sys.stderr)
-        sys.exit(1)
+        return 1
 
     # Check if there is a filter for ipynb files
     if path.exists(attrfile):
@@ -351,7 +351,7 @@ def status(git_config, install_location=INSTALL_LOCATION_LOCAL, verbose=False):
         return 1
 
 
-def main():
+def setup_commandline() -> Namespace:
     parser = ArgumentParser(epilog=__doc__, formatter_class=RawDescriptionHelpFormatter)
     task = parser.add_mutually_exclusive_group()
     task.add_argument('--dry-run', action='store_true',
@@ -403,7 +403,12 @@ def main():
 
     parser.add_argument('files', nargs='*', help='Files to strip output from')
 
-    args = merge_configuration_file(parser.parse_args())
+    return parser
+
+
+def main():
+    parser = setup_commandline()
+    args = merge_configuration_file(parser)
 
     git_config = ['git', 'config']
 
@@ -418,16 +423,16 @@ def main():
         install_location = INSTALL_LOCATION_LOCAL
 
     if args.install:
-        sys.exit(install(git_config, install_location, attrfile=args.attributes))
+        raise SystemExit(install(git_config, install_location, attrfile=args.attributes))
     if args.uninstall:
-        sys.exit(uninstall(git_config, install_location, attrfile=args.attributes))
+        raise SystemExit(uninstall(git_config, install_location, attrfile=args.attributes))
     if args.is_installed:
-        sys.exit(status(git_config, install_location, verbose=False))
+        raise SystemExit(status(git_config, install_location, verbose=False))
     if args.status:
-        sys.exit(status(git_config, install_location, verbose=True))
+        raise SystemExit(status(git_config, install_location, verbose=True))
     if args.version:
         print(__version__)
-        sys.exit(0)
+        raise SystemExit(0)
 
     extra_keys = [
         'metadata.signature',
@@ -493,10 +498,10 @@ def main():
                         write(nb, f)
         except NotJSONError:
             print(f"'{filename}' is not a valid notebook", file=sys.stderr)
-            sys.exit(1)
+            raise SystemExit(1)
         except FileNotFoundError:
             print(f"Could not strip '{filename}': file not found", file=sys.stderr)
-            sys.exit(1)
+            raise SystemExit(1)
         except Exception:
             # Ignore exceptions for non-notebook files.
             print(f"Could not strip '{filename}'", file=sys.stderr)
@@ -507,13 +512,13 @@ def main():
             if args.mode == 'zeppelin':
                 if args.dry_run:
                     output_stream.write('Dry run: would have stripped input from stdin\n')
-                    sys.exit(0)
+                    raise SystemExit(0)
                 nb = json.load(input_stream, object_pairs_hook=collections.OrderedDict)
                 nb_stripped = strip_zeppelin_output(nb)
                 json.dump(nb_stripped, output_stream, indent=2)
                 output_stream.write('\n')
                 output_stream.flush()
-                sys.exit(0)
+                raise SystemExit(0)
             with warnings.catch_warnings():
                 warnings.simplefilter("ignore", category=UserWarning)
                 nb = read(input_stream, as_version=NO_CONVERT)
@@ -532,4 +537,4 @@ def main():
                 output_stream.flush()
         except NotJSONError:
             print('No valid notebook detected', file=sys.stderr)
-            sys.exit(1)
+            raise SystemExit(1)

nbstripout/_utils.py

@@ -1,7 +1,8 @@
-from argparse import Namespace
+from argparse import ArgumentParser, Namespace, _StoreTrueAction, _StoreFalseAction
 import os
 import sys
 from collections import defaultdict
+from functools import partial
 from typing import Any, Dict, Optional
 
 __all__ = ["pop_recursive", "strip_output", "strip_zeppelin_output", "MetadataError"]
@@ -158,18 +159,30 @@ def strip_output(nb, keep_output, keep_count, extra_keys=[], drop_empty_cells=Fa
     return nb
 
 
-def process_pyproject_toml(toml_file_path: str) -> Optional[Dict[str, Any]]:
+def process_pyproject_toml(toml_file_path: str, parser: ArgumentParser) -> Optional[Dict[str, Any]]:
     """Extract config mapping from pyproject.toml file."""
     try:
         import tomllib  # python 3.11+
     except ModuleNotFoundError:
         import tomli as tomllib
 
     with open(toml_file_path, 'rb') as f:
-        return tomllib.load(f).get('tool', {}).get('nbstripout', None)
+        dict_config = tomllib.load(f).get('tool', {}).get('nbstripout', None)
 
+    if not dict_config:
+        # could be {} if 'tool' not found, or None if 'nbstripout' not found
+        return dict_config
 
-def process_setup_cfg(cfg_file_path) -> Optional[Dict[str, Any]]:
+    # special processing of boolean options, make sure we don't have invalid types
+    for a in parser._actions:
+        if a.dest in dict_config and isinstance(a, (_StoreTrueAction, _StoreFalseAction)):
+            if not isinstance(dict_config[a.dest], bool):
+                raise ValueError(f'Argument {a.dest} in pyproject.toml must be a boolean, not {dict_config[a.dest]}')
+
+    return dict_config
+
+
+def process_setup_cfg(cfg_file_path, parser: ArgumentParser) -> Optional[Dict[str, Any]]:
     """Extract config mapping from setup.cfg file."""
     import configparser
 
@@ -178,28 +191,37 @@ def process_setup_cfg(cfg_file_path) -> Optional[Dict[str, Any]]:
     if not reader.has_section('nbstripout'):
         return None
 
-    return reader['nbstripout']
+    raw_config = reader['nbstripout']
+    dict_config = dict(raw_config)
+
+    # special processing of boolean options, to convert various configparser bool types to true/false
+    for a in parser._actions:
+        if a.dest in raw_config and isinstance(a, (_StoreTrueAction, _StoreFalseAction)):
+            dict_config[a.dest] = raw_config.getboolean(a.dest)
+
+    return dict_config
 
 
-def merge_configuration_file(args: Namespace) -> Namespace:
+def merge_configuration_file(parser: ArgumentParser, args_str=None) -> Namespace:
     """Merge flags from config files into args."""
     CONFIG_FILES = {
-        'pyproject.toml': process_pyproject_toml,
-        'setup.cfg': process_setup_cfg,
-    }
-    BOOL_TYPES = {
-        'yes': True,
-        'true': True,
-        'on': True,
-        'no': False,
-        'false': False,
-        'off': False
+        'pyproject.toml': partial(process_pyproject_toml, parser=parser),
+        'setup.cfg': partial(process_setup_cfg, parser=parser),
     }
 
+    # parse args as-is to look for configuration files
+    args = parser.parse_args(args_str)
+
     # Traverse the file tree common to all files given as argument looking for
     # a configuration file
-    config_path = os.path.commonpath([os.path.abspath(file) for file in args.files]) if args.files else os.getcwd()
-    config = None
+    # TODO: make this more like Black:
+    #       By default Black looks for pyproject.toml starting from the common base directory of all files and
+    #       directories passed on the command line. If it’s not there, it looks in parent directories. It stops looking
+    #       when it finds the file, or a .git directory, or a .hg directory, or the root of the file system, whichever
+    #       comes first.
+    # if no files are given, start from cwd
+    config_path = os.path.commonpath([os.path.abspath(file) for file in args.files]) if args.files else os.path.abspath(os.getcwd())
+    config: Optional[Dict[str, Any]] = None
     while True:
         for config_file, processor in CONFIG_FILES.items():
             config_file_path = os.path.join(config_path, config_file)
@@ -213,40 +235,28 @@ def merge_configuration_file(args: Namespace) -> Namespace:
         if not tail:
             break
 
+    # black starts with default arguments (from click), updates that with the config file,
+    # then applies command line arguments. this all happens in the click framework, before main() is called
+    # we can use parser.set_defaults
     if config:
-        # merge config
-        for name, value in config.items():
-            if value is None:
-                continue
-            # additive string flags
-            if name in {'extra_keys'}:
-                args.extra_keys = f"{getattr(args, 'extra_keys', '')} {value}".strip()
-            # singular string flags
-            elif name in {'mode'}:
-                args.mode = value
-            # integer flags
-            elif name in {'max_size'}:
-                args.max_size = int(value)
-            # boolean flags
-            elif name in {
-                'dry_run',
-                'keep_count',
-                'keep_output',
-                'drop_empty_cells',
-                'drop_tagged_cells',
-                'strip_init_cells',
-                '_global',
-                '_system',
-                'force',
-                'textconv',
-            }:
-                if isinstance(value, str):
-                    value = BOOL_TYPES.get(value, value)
-                if not isinstance(value, bool):
-                    raise ValueError(f"Invalid value for {name}: {value}, expected bool")
-                if value:
-                    setattr(args, name.replace('-', '_'), value)
-            else:
+        # check all arguments are valid
+        valid_args = vars(args).keys()
+        for name in config.keys():
+            if name not in valid_args:
                 raise ValueError(f'{name} in the config file is not a valid option')
 
+        # separate into default-overrides and special treatment
+        extra_keys: Optional[str] = None
+        if 'extra_keys' in config:
+            extra_keys = config['extra_keys']
+            del config['extra_keys']
+
+        # merge the configuration options as new defaults, and re-parse the arguments
+        parser.set_defaults(**config)
+        args = parser.parse_args(args_str)
+
+        # merge extra_keys using set union
+        if extra_keys:
+            args.extra_keys = ' '.join(sorted(set(extra_keys.split()) | set(args.extra_keys.split())))
+
     return args

pre-commit

@@ -13,12 +13,12 @@ else
    # Initial commit: diff against an empty tree object
    against=4b825dc642cb6eb9a060e54bf8d69288fbee4904
 fi
- 
+
 # Find notebooks to be committed
 (
 IFS='
 '
-NBS=`git diff-index -z --cached $against --name-only | grep '.ipynb$' | uniq`
+NBS=$(git diff-index -z --cached $against --name-only | grep '.ipynb$' | uniq)
 
 for NB in $NBS ; do
     echo "Removing outputs from $NB"

tests/conftest.py

@@ -0,0 +1 @@
+pytest_plugins = "pytester"