From eb97ead7d14e968705e7eece8399cea72b2ef785 Mon Sep 17 00:00:00 2001 From: Kay Robbins <1189050+VisLab@users.noreply.github.com> Date: Tue, 20 Jan 2026 15:27:07 -0600 Subject: [PATCH 1/2] Worked on making script logging more uniform --- hed/cli/cli.py | 49 ++++++++++++++++----- hed/scripts/extract_tabular_summary.py | 58 +++---------------------- hed/scripts/hed_extract_bids_sidecar.py | 37 ++-------------- hed/scripts/validate_bids.py | 2 +- 4 files changed, 48 insertions(+), 98 deletions(-) diff --git a/hed/cli/cli.py b/hed/cli/cli.py index d709715b..11830c39 100644 --- a/hed/cli/cli.py +++ b/hed/cli/cli.py @@ -191,7 +191,9 @@ def validate(): is_flag=True, help="Disable all logging output", ) +@click.pass_context def validate_bids_cmd( + ctx, data_path, error_limit, errors_by_file, @@ -244,7 +246,8 @@ def validate_bids_cmd( args.append("-x") args.extend(exclude_dirs) - validate_bids_main(args) + result = validate_bids_main(args) + ctx.exit(result if result is not None else 0) @validate.command( @@ -354,7 +357,9 @@ def validate_bids_cmd( is_flag=True, help="Disable all logging output", ) +@click.pass_context def validate_hed_string_cmd( + ctx, hed_string, schema_version, definitions, @@ -395,14 +400,16 @@ def validate_hed_string_cmd( if verbose: args.append("-v") - validate_string_main(args) + result = validate_string_main(args) + ctx.exit(result if result is not None else 0) @schema.command(name="validate") @click.argument("schema_path", type=click.Path(exists=True), nargs=-1, required=True) @click.option("--add-all-extensions", is_flag=True, help="Always verify all versions of the same schema are equal") @click.option("-v", "--verbose", is_flag=True, help="Enable verbose output") -def schema_validate_cmd(schema_path, add_all_extensions, verbose): +@click.pass_context +def schema_validate_cmd(ctx, schema_path, add_all_extensions, verbose): """Validate HED schema files. This command validates HED schema files for correctness, checking structure, @@ -434,13 +441,15 @@ def schema_validate_cmd(schema_path, add_all_extensions, verbose): if verbose: args.append("-v") - validate_schemas_main(args) + result = validate_schemas_main(args) + ctx.exit(result if result is not None else 0) @schema.command(name="convert") @click.argument("schema_path", type=click.Path(exists=True), nargs=-1, required=True) @click.option("--set-ids", is_flag=True, help="Set/update HED IDs in the schema") -def schema_convert_cmd(schema_path, set_ids): +@click.pass_context +def schema_convert_cmd(ctx, schema_path, set_ids): """Convert HED schema between formats (TSV, XML, MEDIAWIKI, JSON). This command converts HED schema files between different formats while @@ -467,14 +476,16 @@ def schema_convert_cmd(schema_path, set_ids): if set_ids: args.append("--set-ids") - convert_main(args) + result = convert_main(args) + ctx.exit(result if result is not None else 0) @schema.command(name="add-ids") @click.argument("repo_path", type=click.Path(exists=True)) @click.argument("schema_name") @click.argument("schema_version") -def schema_add_ids_cmd(repo_path, schema_name, schema_version): +@click.pass_context +def schema_add_ids_cmd(ctx, repo_path, schema_name, schema_version): """Add HED IDs to a schema. This command adds unique HED IDs to schema elements that don't have them, @@ -498,7 +509,8 @@ def schema_add_ids_cmd(repo_path, schema_name, schema_version): args = [repo_path, schema_name, schema_version] - add_ids_main(args) + result = add_ids_main(args) + ctx.exit(result if result is not None else 0) @cli.group() @@ -613,8 +625,19 @@ def extract(): is_flag=True, help="Suppress log output to stderr; only applicable when --log-file is used (logs go only to file)", ) +@click.pass_context def extract_bids_sidecar_cmd( - data_path, suffix, value_columns, skip_columns, log_level, log_file, log_quiet, output_file, verbose, exclude_dirs + ctx, + data_path, + suffix, + value_columns, + skip_columns, + log_level, + log_file, + log_quiet, + output_file, + verbose, + exclude_dirs, ): """Extract a sidecar template from a BIDS dataset. @@ -643,7 +666,8 @@ def extract_bids_sidecar_cmd( args.append("-x") args.extend(exclude_dirs) - extract_main(args) + result = extract_main(args) + ctx.exit(result if result is not None else 0) @extract.command( @@ -781,7 +805,9 @@ def extract_bids_sidecar_cmd( is_flag=True, help="Suppress log output to stderr; only applicable when --log-file is used (logs go only to file)", ) +@click.pass_context def extract_tabular_summary_cmd( + ctx, data_path, name_prefix, name_suffix, @@ -834,7 +860,8 @@ def extract_tabular_summary_cmd( if verbose: args.append("-v") - extract_summary_main(args) + result = extract_summary_main(args) + ctx.exit(result if result is not None else 0) def main(): diff --git a/hed/scripts/extract_tabular_summary.py b/hed/scripts/extract_tabular_summary.py index dcc5ff97..0481bc21 100644 --- a/hed/scripts/extract_tabular_summary.py +++ b/hed/scripts/extract_tabular_summary.py @@ -45,6 +45,7 @@ from hed import _version as vr from hed.tools.util.io_util import get_file_list from hed.tools.analysis.tabular_summary import TabularSummary +from hed.scripts.script_utils import setup_logging def get_parser(): @@ -338,57 +339,6 @@ def format_output(summary, args): return json.dumps(output_dict, indent=4) -def setup_logging(args): - """Configure logging based on command line arguments. - - Parameters: - args (argparse.Namespace): Parsed command line arguments. - - Returns: - logging.Logger: Configured logger instance. - """ - # Determine log level - log_level = args.log_level.upper() if args.log_level else "WARNING" - if args.verbose: - log_level = "INFO" - - # Configure logging format - log_format = "%(asctime)s - %(name)s - %(levelname)s - %(message)s" - date_format = "%Y-%m-%d %H:%M:%S" - - # Clear any existing handlers from root logger - root_logger = logging.getLogger() - for handler in root_logger.handlers[:]: - root_logger.removeHandler(handler) - - # Set the root logger level - root_logger.setLevel(getattr(logging, log_level)) - - # Create formatter - formatter = logging.Formatter(log_format, datefmt=date_format) - - # File handler if log file specified - if args.log_file: - file_handler = logging.FileHandler(args.log_file, mode="w", encoding="utf-8") - file_handler.setLevel(getattr(logging, log_level)) - file_handler.setFormatter(formatter) - root_logger.addHandler(file_handler) - - # Console handler (stderr) unless explicitly quieted and file logging is used - if not args.log_quiet or not args.log_file: - console_handler = logging.StreamHandler(sys.stderr) - console_handler.setLevel(getattr(logging, log_level)) - console_handler.setFormatter(formatter) - root_logger.addHandler(console_handler) - - logger = logging.getLogger("extract_tabular_summary") - logger.info(f"Starting tabular summary extraction with log level: {log_level}") - if args.log_file: - logger.info(f"Log output will be saved to: {args.log_file}") - - return logger - - def main(arg_list=None): """Main entry point for the script. @@ -406,7 +356,11 @@ def main(arg_list=None): args = parser.parse_args(arg_list) # Setup logging - logger = setup_logging(args) + setup_logging(args.log_level, args.log_file, args.log_quiet, args.verbose, False) + logger = logging.getLogger("extract_tabular_summary") + logger.info(f"Starting tabular summary extraction with log level: {args.log_level}") + if args.log_file: + logger.info(f"Log output will be saved to: {args.log_file}") try: # Extract the summary diff --git a/hed/scripts/hed_extract_bids_sidecar.py b/hed/scripts/hed_extract_bids_sidecar.py index 5f65504a..543c17ba 100644 --- a/hed/scripts/hed_extract_bids_sidecar.py +++ b/hed/scripts/hed_extract_bids_sidecar.py @@ -32,9 +32,9 @@ import argparse import json import logging -import sys from hed import _version as vr from hed.tools import BidsDataset +from hed.scripts.script_utils import setup_logging def get_parser(): @@ -219,41 +219,10 @@ def main(arg_list=None): args = parser.parse_args(arg_list) # Setup logging configuration - log_level = args.log_level.upper() if args.log_level else "WARNING" - if args.verbose: - log_level = "INFO" - - # Configure logging format - log_format = "%(asctime)s - %(name)s - %(levelname)s - %(message)s" - date_format = "%Y-%m-%d %H:%M:%S" - - # Clear any existing handlers from root logger - root_logger = logging.getLogger() - for handler in root_logger.handlers[:]: - root_logger.removeHandler(handler) - - # Set the root logger level - this is crucial for filtering - root_logger.setLevel(getattr(logging, log_level)) - - # Create and configure handlers - formatter = logging.Formatter(log_format, datefmt=date_format) - - # File handler if log file specified - if args.log_file: - file_handler = logging.FileHandler(args.log_file, mode="w", encoding="utf-8") - file_handler.setLevel(getattr(logging, log_level)) - file_handler.setFormatter(formatter) - root_logger.addHandler(file_handler) - - # Console handler (stderr) unless explicitly quieted and file logging is used - if not args.log_quiet or not args.log_file: - console_handler = logging.StreamHandler(sys.stderr) - console_handler.setLevel(getattr(logging, log_level)) - console_handler.setFormatter(formatter) - root_logger.addHandler(console_handler) + setup_logging(args.log_level, args.log_file, args.log_quiet, args.verbose, False) logger = logging.getLogger("extract_bids_sidecar") - logger.info(f"Starting BIDS sidecar extraction with log level: {log_level}") + logger.info(f"Starting BIDS sidecar extraction with log level: {args.log_level}") if args.log_file: logger.info(f"Log output will be saved to: {args.log_file}") diff --git a/hed/scripts/validate_bids.py b/hed/scripts/validate_bids.py index bf6312d8..3247177e 100644 --- a/hed/scripts/validate_bids.py +++ b/hed/scripts/validate_bids.py @@ -201,12 +201,12 @@ def main(arg_list=None): # Parse the arguments args = parser.parse_args(arg_list) - print(f"{str(args)}") # Set up logging setup_logging(args.log_level, args.log_file, args.log_quiet, args.verbose, args.no_log) logger = logging.getLogger("validate_bids") + logger.debug("Parsed arguments: %s", args) logger.info(f"Starting BIDS validation with log level: {args.log_level}") if args.log_file: logger.info(f"Log output will be saved to: {args.log_file}") From e32546b9c12f05b291e295319795457c54f7e3a2 Mon Sep 17 00:00:00 2001 From: Kay Robbins <1189050+VisLab@users.noreply.github.com> Date: Wed, 21 Jan 2026 06:59:52 -0600 Subject: [PATCH 2/2] Addressed copilot review --- hed/scripts/extract_tabular_summary.py | 6 +++++- hed/scripts/hed_convert_schema.py | 4 ++-- hed/scripts/hed_extract_bids_sidecar.py | 7 ++++++- hed/scripts/validate_bids.py | 8 +++++++- hed/scripts/validate_hed_string.py | 9 +++++++++ hed/scripts/validate_schemas.py | 4 ++-- 6 files changed, 31 insertions(+), 7 deletions(-) diff --git a/hed/scripts/extract_tabular_summary.py b/hed/scripts/extract_tabular_summary.py index 0481bc21..d2a340f1 100644 --- a/hed/scripts/extract_tabular_summary.py +++ b/hed/scripts/extract_tabular_summary.py @@ -358,7 +358,11 @@ def main(arg_list=None): # Setup logging setup_logging(args.log_level, args.log_file, args.log_quiet, args.verbose, False) logger = logging.getLogger("extract_tabular_summary") - logger.info(f"Starting tabular summary extraction with log level: {args.log_level}") + effective_level = logging.getLevelName(logger.getEffectiveLevel()) + logger.info( + f"Starting tabular summary extraction with effective log level: {effective_level} " + f"(requested: {args.log_level}, verbose={'on' if args.verbose else 'off'})" + ) if args.log_file: logger.info(f"Log output will be saved to: {args.log_file}") diff --git a/hed/scripts/hed_convert_schema.py b/hed/scripts/hed_convert_schema.py index 3589d033..720b2693 100644 --- a/hed/scripts/hed_convert_schema.py +++ b/hed/scripts/hed_convert_schema.py @@ -72,12 +72,12 @@ def convert_and_update(filenames, set_ids): return 0 -def main(): +def main(arg_list=None): parser = argparse.ArgumentParser(description="Update other schema formats based on the changed one.") parser.add_argument("filenames", nargs="*", help="List of files to process") parser.add_argument("--set-ids", action="store_true", help="Add missing HED ids") - args = parser.parse_args() + args = parser.parse_args(arg_list) filenames = args.filenames set_ids = args.set_ids diff --git a/hed/scripts/hed_extract_bids_sidecar.py b/hed/scripts/hed_extract_bids_sidecar.py index 543c17ba..1aec0a24 100644 --- a/hed/scripts/hed_extract_bids_sidecar.py +++ b/hed/scripts/hed_extract_bids_sidecar.py @@ -32,6 +32,7 @@ import argparse import json import logging +import sys from hed import _version as vr from hed.tools import BidsDataset from hed.scripts.script_utils import setup_logging @@ -222,7 +223,11 @@ def main(arg_list=None): setup_logging(args.log_level, args.log_file, args.log_quiet, args.verbose, False) logger = logging.getLogger("extract_bids_sidecar") - logger.info(f"Starting BIDS sidecar extraction with log level: {args.log_level}") + effective_level = logging.getLevelName(logger.getEffectiveLevel()) + logger.info( + f"Starting BIDS sidecar extraction with effective log level: {effective_level} " + f"(requested: {args.log_level}, verbose={'on' if args.verbose else 'off'})" + ) if args.log_file: logger.info(f"Log output will be saved to: {args.log_file}") diff --git a/hed/scripts/validate_bids.py b/hed/scripts/validate_bids.py index 3247177e..85246d11 100644 --- a/hed/scripts/validate_bids.py +++ b/hed/scripts/validate_bids.py @@ -207,7 +207,13 @@ def main(arg_list=None): logger = logging.getLogger("validate_bids") logger.debug("Parsed arguments: %s", args) - logger.info(f"Starting BIDS validation with log level: {args.log_level}") + effective_level_name = logging.getLevelName(logger.getEffectiveLevel()) + logger.info( + "Starting BIDS validation with effective log level: %s (requested: %s, verbose=%s)", + effective_level_name, + args.log_level, + "on" if args.verbose else "off", + ) if args.log_file: logger.info(f"Log output will be saved to: {args.log_file}") diff --git a/hed/scripts/validate_hed_string.py b/hed/scripts/validate_hed_string.py index a0813ff5..a1976b53 100644 --- a/hed/scripts/validate_hed_string.py +++ b/hed/scripts/validate_hed_string.py @@ -98,6 +98,15 @@ def main(arg_list=None): import logging + logger = logging.getLogger("validate_hed_string") + effective_level_name = logging.getLevelName(logger.getEffectiveLevel()) + logger.info( + "Starting HED string validation with effective log level: %s (requested: %s, verbose=%s)", + effective_level_name, + args.log_level, + "on" if args.verbose else "off", + ) + try: # Load schema (handle single version or list of versions) schema_versions = args.schema_version[0] if len(args.schema_version) == 1 else args.schema_version diff --git a/hed/scripts/validate_schemas.py b/hed/scripts/validate_schemas.py index 8ecc28d3..2abd1fc5 100644 --- a/hed/scripts/validate_schemas.py +++ b/hed/scripts/validate_schemas.py @@ -19,9 +19,9 @@ def get_parser(): return parser -def main(): +def main(arg_list=None): parser = get_parser() - args = parser.parse_args() + args = parser.parse_args(arg_list) schema_files = sort_base_schemas(args.schema_files, args.add_all_extensions) issues = validate_all_schemas(schema_files)