Logger with file by sahiljhawar · Pull Request #54 · GFZ/SWVO

sahiljhawar · 2026-02-13T10:09:51Z

No description provided.

Copilot

Pull request overview

This PR enhances the logging configuration to support file output in addition to console logging. The changes modify setup_logging() to accept a file path parameter and configure the root logger instead of just the package logger.

Changes:

Modified setup_logging() signature to accept string log levels, optional file path, and file mode
Changed logging configuration from package logger to root logger
Added file handler support with configurable write mode
Enhanced log format to include line numbers

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-13T10:14:08Z

+    level : str, optional
+        Logging level, by default is INFO
+    log_file : Path, optional
+        Path to log file. If None, only console logging is enabled.If provided, logs will be written to both console and file., by default None


Missing space after period in the docstring. The text "enabled.If" should be "enabled. If" with a space separating the two sentences.

Suggested change

Path to log file. If None, only console logging is enabled.If provided, logs will be written to both console and file., by default None

Path to log file. If None, only console logging is enabled. If provided, logs will be written to both console and file., by default None

Copilot · 2026-02-13T10:14:09Z

+        Logging level, by default is INFO
+    log_file : Path, optional
+        Path to log file. If None, only console logging is enabled.If provided, logs will be written to both console and file., by default None


The docstring is missing documentation for the file_mode parameter. All parameters should be documented in the Parameters section, including file_mode which controls whether the log file is overwritten ("w") or appended to ("a").

Suggested change

Logging level, by default is INFO

log_file : Path, optional

Path to log file. If None, only console logging is enabled.If provided, logs will be written to both console and file., by default None

Logging level, by default is INFO.

log_file : Path, optional

Path to log file. If None, only console logging is enabled. If provided,

logs will be written to both console and file. By default None.

file_mode : str, optional

File open mode for the log file: ``"w"`` to overwrite or ``"a"`` to append.

By default ``"w"``.

Copilot · 2026-02-13T10:14:09Z

+    if any(isinstance(h, logging.StreamHandler) for h in root_logger.handlers):
        return


The check for existing configuration now looks at the root logger instead of the package logger. This changes the semantics of the function - it will now skip configuration if ANY StreamHandler exists on the root logger, even if it wasn't added by this function. This could prevent proper setup if another library has already configured the root logger. The previous implementation checked logger.handlers (the "swvo" package logger), which was more isolated.

Copilot · 2026-02-13T10:14:09Z

-    logger.addHandler(handler)
+
+    # Console handler with colors
+    log_format = "[%(levelname)-8s] %(asctime)s - %(name)s:%(lineno)d - %(message)s"


The log format has been changed to include line numbers (:%(lineno)d). While this can be useful for debugging, it represents a breaking change in the log output format that may affect any systems or scripts that parse the log files. This change should be documented in the PR description or changelog.

Suggested change

log_format = "[%(levelname)-8s] %(asctime)s - %(name)s:%(lineno)d - %(message)s"

log_format = "[%(levelname)-8s] %(asctime)s - %(name)s - %(message)s"

Copilot · 2026-02-13T10:14:10Z

+    if log_file:
+        file_handler = logging.FileHandler(log_file, mode=file_mode)
+        file_handler.setFormatter(logging.Formatter(log_format, datefmt=datefmt))
+        root_logger.addHandler(file_handler)


There is no test coverage for the new file logging functionality. Tests should be added to verify that when log_file is provided, a FileHandler is correctly added to the root logger with the proper formatter and file mode. Additionally, tests should verify that the file_mode parameter works correctly (e.g., testing both "w" and "a" modes).

Copilot · 2026-02-13T10:14:10Z

+    root_logger.addHandler(console_handler)
+
+    if log_file:
+        file_handler = logging.FileHandler(log_file, mode=file_mode)


The file_mode parameter is not validated. Invalid values (e.g., "x", "r", or arbitrary strings) will be passed directly to FileHandler(), which could cause runtime errors or unexpected behavior. Consider adding validation to ensure file_mode is one of the valid modes ("w", "a", "x") or documenting the acceptable values in the docstring.

Copilot · 2026-02-13T10:14:10Z

+def setup_logging(level: str = "INFO", log_file: Optional[Path] = None, file_mode: str = "w"):
+    """Setup logging for the swvo package and root logger.
+
+    Parameters
+    ----------
+    level : str, optional
+        Logging level, by default is INFO
+    log_file : Path, optional
+        Path to log file. If None, only console logging is enabled.If provided, logs will be written to both console and file., by default None
+    """
+    try:
+        level = getattr(logging, level.upper())
+    except AttributeError:
+        raise ValueError(f"Invalid logging level: {level}. Use one of DEBUG, INFO, WARNING, ERROR, CRITICAL.")


The function signature has changed from accepting a logging level constant (e.g., logging.INFO) to accepting a string (e.g., "INFO"). This is a breaking change that will cause the existing call setup_logging() in download_daily_files.py to continue working with the default, but any existing code that passes a logging level constant like setup_logging(logging.DEBUG) will fail. The old signature was level=logging.INFO (an int constant), and the new signature is level: str = "INFO" (a string).

Suggested change

def setup_logging(level: str = "INFO", log_file: Optional[Path] = None, file_mode: str = "w"):

"""Setup logging for the swvo package and root logger.

Parameters

----------

level : str, optional

Logging level, by default is INFO

log_file : Path, optional

Path to log file. If None, only console logging is enabled.If provided, logs will be written to both console and file., by default None

"""

try:

level = getattr(logging, level.upper())

except AttributeError:

raise ValueError(f"Invalid logging level: {level}. Use one of DEBUG, INFO, WARNING, ERROR, CRITICAL.")

def setup_logging(level=logging.INFO, log_file: Optional[Path] = None, file_mode: str = "w"):

"""Setup logging for the swvo package and root logger.

Parameters

----------

level : int or str, optional

Logging level. Can be a logging level constant (e.g., logging.INFO) or a string

(e.g., "INFO"). By default is INFO.

log_file : Path, optional

Path to log file. If None, only console logging is enabled.If provided, logs will be written to both console and file., by default None

"""

if isinstance(level, str):

try:

level = getattr(logging, level.upper())

except AttributeError:

raise ValueError(

f"Invalid logging level: {level}. Use one of DEBUG, INFO, WARNING, ERROR, CRITICAL."

)

elif not isinstance(level, int):

raise ValueError(

f"Invalid logging level type: {type(level)!r}. Expected int or str."

)

Copilot · 2026-02-13T10:14:10Z

+    root_logger.setLevel(level)
+    root_logger.addHandler(console_handler)


The existing test test_setup_logging_adds_stream_handler checks that a StreamHandler is added to the logger (the "swvo" package logger), but the new implementation adds handlers to the root logger instead. This test will fail because it's checking the wrong logger's handlers. The test needs to be updated to check logging.getLogger().handlers instead of logger.handlers.

Copilot · 2026-02-13T10:14:11Z

+    root_logger.addHandler(console_handler)
+
+    if log_file:
+        file_handler = logging.FileHandler(log_file, mode=file_mode)


The file handler does not include color formatting, while the console handler uses _ColorFormatter. This is intentional and correct since ANSI color codes should not be written to log files. However, consider adding a comment explaining why different formatters are used for console vs. file handlers to improve code maintainability.

Suggested change

file_handler = logging.FileHandler(log_file, mode=file_mode)

file_handler = logging.FileHandler(log_file, mode=file_mode)

# Use a non-colored formatter for files to avoid writing ANSI escape codes to log files.

sahiljhawar added 3 commits February 13, 2026 10:52

refactor: use API to fetch Hp index

2476aec

fix(logger): use a root logger and log_file in setup_logging

8e55164

Merge branch 'main' into logger-with-file

6198ab0

Copilot AI review requested due to automatic review settings February 13, 2026 10:09

Copilot started reviewing on behalf of sahiljhawar February 13, 2026 10:10 View session

sahiljhawar merged commit 72b9611 into GFZ:main Feb 13, 2026
5 of 12 checks passed

Copilot AI reviewed Feb 13, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Logger with file#54

Logger with file#54
sahiljhawar merged 3 commits into
GFZ:mainfrom
sahiljhawar:logger-with-file

sahiljhawar commented Feb 13, 2026

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 13, 2026

Uh oh!

Copilot AI Feb 13, 2026

Uh oh!

Copilot AI Feb 13, 2026

Uh oh!

Copilot AI Feb 13, 2026

Uh oh!

Copilot AI Feb 13, 2026

Uh oh!

Copilot AI Feb 13, 2026

Uh oh!

Copilot AI Feb 13, 2026

Uh oh!

Copilot AI Feb 13, 2026

Uh oh!

Copilot AI Feb 13, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	Path to log file. If None, only console logging is enabled.If provided, logs will be written to both console and file., by default None
	Path to log file. If None, only console logging is enabled. If provided, logs will be written to both console and file., by default None

-        Logging level, by default is INFO
-    log_file : Path, optional
-        Path to log file. If None, only console logging is enabled.If provided, logs will be written to both console and file., by default None
+        Logging level, by default is INFO.
+    log_file : Path, optional
+        Path to log file. If None, only console logging is enabled. If provided,
+        logs will be written to both console and file. By default None.
+    file_mode : str, optional
+        File open mode for the log file: ``"w"`` to overwrite or ``"a"`` to append.
+        By default ``"w"``.

		if any(isinstance(h, logging.StreamHandler) for h in root_logger.handlers):
		return

	log_format = "[%(levelname)-8s] %(asctime)s - %(name)s:%(lineno)d - %(message)s"
	log_format = "[%(levelname)-8s] %(asctime)s - %(name)s - %(message)s"

		root_logger.setLevel(level)
		root_logger.addHandler(console_handler)

	file_handler = logging.FileHandler(log_file, mode=file_mode)
	file_handler = logging.FileHandler(log_file, mode=file_mode)
	# Use a non-colored formatter for files to avoid writing ANSI escape codes to log files.

Conversation

sahiljhawar commented Feb 13, 2026

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Feb 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 13, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants