Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support config files #169

Closed
wants to merge 17 commits into from
Closed
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
fix handling of non_bool values in pyproject.toml
Andrew O'Brien committed Dec 8, 2022
commit 92440f1d12605f74584e5d4721e1bca6bb01a869
18 changes: 15 additions & 3 deletions nbstripout/_utils.py
Original file line number Diff line number Diff line change
@@ -159,15 +159,27 @@ 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

# 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)):
Comment on lines +178 to +179
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a big fan of reaching into the internals of ArgumentParser but also don't have a great idea what to do instead.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I thought that this would still be more robust than keeping an explicit list of Boolean arguments, although that wouldn't be impossible. I looked at a range of options, including shifting to an alternative arguments library, like Click, but that all got too complicated.
I think that this is a reasonable compromise if we keep on top of adding new Python versions to the test suite whenever they become available.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, agree

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]]:
@@ -193,7 +205,7 @@ def process_setup_cfg(cfg_file_path, parser: ArgumentParser) -> Optional[Dict[st
def merge_configuration_file(parser: ArgumentParser, args_str=None) -> Namespace:
"""Merge flags from config files into args."""
CONFIG_FILES = {
'pyproject.toml': process_pyproject_toml,
'pyproject.toml': partial(process_pyproject_toml, parser=parser),
'setup.cfg': partial(process_setup_cfg, parser=parser),
}

2 changes: 2 additions & 0 deletions tests/test_read_config_files.py
Original file line number Diff line number Diff line change
@@ -82,6 +82,7 @@ def test_non_nested_setup_cfg_non_empty(tmp_path: Path, test_nb: Path, parser: A


def test_nofiles_pyproject_toml_non_empty(tmp_path: Path, parser: ArgumentParser) -> None:
pytest.skip("TODO")
(tmp_path / "pyproject.toml").write_text(
"[tool.nbstripout]\ndrop_empty_cells=true\n",
)
@@ -93,6 +94,7 @@ def test_nofiles_pyproject_toml_non_empty(tmp_path: Path, parser: ArgumentParser


def test_nofiles_setup_cfg_empty(tmp_path: Path, test_nb: Path, parser: ArgumentParser) -> None:
pytest.skip("TODO")
(tmp_path / "setup.cfg").write_text(
"[nbstripout]\ndrop_empty_cells = yes\n",
)