Skip to content

Commit

Permalink
refactor(run_modules): Move common setup to individual functions
Browse files Browse the repository at this point in the history 
Each `run_<module>()` will still need to parse configuration and load files.

Rather than duplicate the code that does so I've broken out the functionality into a few small private
functions (nothing is private in Python land though) so they can be re-used across the `run_<module>()` functions.

Introduces a simple test of logging level, must other functionality already has tests in place.

Aiming to keep changes and PRs small to aid reviewing.
  • Loading branch information
@ns-rse
ns-rse committed Nov 12, 2024
1 parent 835e00f commit 63c167e
Show file tree
Hide file tree
Showing 2 changed files with 102 additions and 35 deletions.
19 changes: 17 additions & 2 deletions tests/test_run_modules.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,8 @@

import pytest

from topostats.entry_point import entry_point
from topostats.logs.logs import LOGGER_NAME
from topostats.run_modules import reconcile_config_args
from topostats.run_modules import _set_logging, reconcile_config_args
from topostats.validation import DEFAULT_CONFIG_SCHEMA, validate_config

BASE_DIR = Path.cwd()
Expand Down Expand Up @@ -81,6 +80,22 @@ def test_reconcile_config_args_partial_config_with_overrides() -> None:
validate_config(config, schema=DEFAULT_CONFIG_SCHEMA, config_type="YAML configuration file")


@pytest.mark.parametrize(
("log_level", "effective_level"),
[
pytest.param("debug", 10, id="log level debug"),
pytest.param("info", 20, id="log level warning"),
pytest.param("warning", 30, id="log level warning"),
pytest.param("error", 40, id="log level error"),
],
)
def test_set_logging(log_level: str, effective_level: int) -> None:
"""Test setting log-level."""
LOGGER = logging.getLogger(LOGGER_NAME)
_set_logging(log_level)
assert LOGGER.getEffectiveLevel() == effective_level


@pytest.mark.parametrize("option", [("-h"), ("--help")])
def test_run_topostats_main_help(capsys, option) -> None:
"""Test the -h/--help flag to run_topostats."""
Expand Down
118 changes: 85 additions & 33 deletions topostats/run_modules.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,7 @@ def reconcile_config_args(args: argparse.Namespace | None) -> dict:
dict
The configuration dictionary.
"""
default_config_raw = (resources.files(__package__) / "default_config.yaml").read_text()
default_config = yaml.safe_load(default_config_raw)
default_config = read_yaml(resources.files(__package__) / "default_config.yaml")
if args is not None:
config_file_arg: str | None = args.config_file
if config_file_arg is not None:
Expand All @@ -103,14 +102,67 @@ def reconcile_config_args(args: argparse.Namespace | None) -> dict:
return config


def process(args: argparse.Namespace | None = None) -> None: # noqa: C901
def _set_logging(log_level: str | None) -> None:
"""
Find and process all files.
Set the logging level.
Parameters
----------
log_level : str
String for the desired log-level.
"""
if log_level == "warning":
LOGGER.setLevel("WARNING")
elif log_level == "error":
LOGGER.setLevel("ERROR")
elif log_level == "debug":
LOGGER.setLevel("DEBUG")
else:
LOGGER.setLevel("INFO")


def _log_setup(config: dict, args: argparse.Namespace | None, img_files: dict) -> None:
"""
Log the current configuration.
Parameters
----------
config : dict
Dictionary of configuration options.
args : argparse.Namespace | None
Arguments function was invoked with.
img_files : dict
Dictionary of image files that have been found.
"""
LOGGER.debug(f"Plotting configuration after update :\n{pformat(config['plotting'], indent=4)}")

LOGGER.info(f"Configuration file loaded from : {args.config_file}")
LOGGER.info(f"Scanning for images in : {config['base_dir']}")
LOGGER.info(f"Output directory : {str(config['output_dir'])}")
LOGGER.info(f"Looking for images with extension : {config['file_ext']}")
LOGGER.info(f"Images with extension {config['file_ext']} in {config['base_dir']} : {len(img_files)}")
if len(img_files) == 0:
LOGGER.error(f"No images with extension {config['file_ext']} in {config['base_dir']}")
LOGGER.error("Please check your configuration and directories.")
sys.exit()
LOGGER.info(f"Thresholding method (Filtering) : {config['filter']['threshold_method']}")
LOGGER.info(f"Thresholding method (Grains) : {config['grains']['threshold_method']}")
LOGGER.debug(f"Configuration after update : \n{pformat(config, indent=4)}") # noqa : T203


def _parse_configuration(args: argparse.Namespace | None = None) -> tuple[dict, dict]:
"""
Load configurations, validate and check run steps are consistent.
Parameters
----------
args : None
Arguments.
Returns
-------
tuple[dict, dict]
Returns the dictionary of configuration options and a dictionary of image files found on the input path.
"""
# Parse command line options, load config (or default) and update with command line options
config = reconcile_config_args(args=args)
Expand All @@ -119,14 +171,7 @@ def process(args: argparse.Namespace | None = None) -> None: # noqa: C901
validate_config(config, schema=DEFAULT_CONFIG_SCHEMA, config_type="YAML configuration file")

# Set logging level
if config["log_level"] == "warning":
LOGGER.setLevel("WARNING")
elif config["log_level"] == "error":
LOGGER.setLevel("ERROR")
elif config["log_level"] == "debug":
LOGGER.setLevel("DEBUG")
else:
LOGGER.setLevel("INFO")
_set_logging(config["log_level"])

# Create base output directory
config["output_dir"].mkdir(parents=True, exist_ok=True)
Expand All @@ -151,21 +196,21 @@ def process(args: argparse.Namespace | None = None) -> None: # noqa: C901
)
# Ensures each image has all plotting options which are passed as **kwargs
config["plotting"] = update_plotting_config(config["plotting"])
LOGGER.debug(f"Plotting configuration after update :\n{pformat(config['plotting'], indent=4)}")

LOGGER.info(f"Configuration file loaded from : {args.config_file}")
LOGGER.info(f"Scanning for images in : {config['base_dir']}")
LOGGER.info(f"Output directory : {str(config['output_dir'])}")
LOGGER.info(f"Looking for images with extension : {config['file_ext']}")
img_files = find_files(config["base_dir"], file_ext=config["file_ext"])
LOGGER.info(f"Images with extension {config['file_ext']} in {config['base_dir']} : {len(img_files)}")
if len(img_files) == 0:
LOGGER.error(f"No images with extension {config['file_ext']} in {config['base_dir']}")
LOGGER.error("Please check your configuration and directories.")
sys.exit()
LOGGER.info(f"Thresholding method (Filtering) : {config['filter']['threshold_method']}")
LOGGER.info(f"Thresholding method (Grains) : {config['grains']['threshold_method']}")
LOGGER.debug(f"Configuration after update : \n{pformat(config, indent=4)}") # noqa : T203
_log_setup(config, args, img_files)
return config, img_files


def process(args: argparse.Namespace | None = None) -> None: # noqa: C901
"""
Find and process all files.
Parameters
----------
args : None
Arguments.
"""
config, img_files = _parse_configuration(args)

processing_function = partial(
process_scan,
Expand Down Expand Up @@ -349,7 +394,7 @@ def process(args: argparse.Namespace | None = None) -> None: # noqa: C901
# pylint: disable=unused-argument


def filters(args: None = None) -> None:
def filters(args: argparse.Namespace | None = None) -> None:
"""
Load files from disk and run filtering.
Expand All @@ -358,10 +403,11 @@ def filters(args: None = None) -> None:
args : None
Arguments.
"""
config, img_files = _parse_configuration(args)
run_filters()


def grains(args: None = None) -> None:
def grains(args: argparse.Namespace | None = None) -> None:
"""
Load files from disk and run grain finding.
Expand All @@ -370,10 +416,11 @@ def grains(args: None = None) -> None:
args : None
Arguments.
"""
config, img_files = _parse_configuration(args)
run_grains()


def grainstats(args: None = None) -> None:
def grainstats(args: argparse.Namespace | None = None) -> None:
"""
Load files from disk and run grainstats.
Expand All @@ -382,10 +429,11 @@ def grainstats(args: None = None) -> None:
args : None
Arguments.
"""
config, img_files = _parse_configuration(args)
run_grainstats()


def disordered_tracing(args: None = None) -> None:
def disordered_tracing(args: argparse.Namespace | None = None) -> None:
"""
Load files from disk and run grainstats.
Expand All @@ -394,10 +442,11 @@ def disordered_tracing(args: None = None) -> None:
args : None
Arguments.
"""
config, img_files = _parse_configuration(args)
run_disordered_tracing()


def nodestats(args: None = None) -> None:
def nodestats(args: argparse.Namespace | None = None) -> None:
"""
Load files from disk and run grainstats.
Expand All @@ -406,10 +455,11 @@ def nodestats(args: None = None) -> None:
args : None
Arguments.
"""
config, img_files = _parse_configuration(args)
run_nodestats()


def ordered_tracing(args: None = None) -> None:
def ordered_tracing(args: argparse.Namespace | None = None) -> None:
"""
Load files from disk and run grainstats.
Expand All @@ -418,10 +468,11 @@ def ordered_tracing(args: None = None) -> None:
args : None
Arguments.
"""
config, img_files = _parse_configuration(args)
run_ordered_tracing()


def splining(args: None = None) -> None:
def splining(args: argparse.Namespace | None = None) -> None:
"""
Load files from disk and run grainstats.
Expand All @@ -430,4 +481,5 @@ def splining(args: None = None) -> None:
args : None
Arguments.
"""
config, img_files = _parse_configuration(args)
run_splining()

0 comments on commit 63c167e

Please sign in to comment.