Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
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
2 changes: 1 addition & 1 deletion package/MDAnalysis/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@
from .lib import log
from .lib.log import start_logging, stop_logging

logging.getLogger("MDAnalysis").addHandler(log.NullHandler())
logging.getLogger("MDAnalysis").addHandler(logging.NullHandler())
del logging

# only MDAnalysis DeprecationWarnings are loud by default
Expand Down
79 changes: 39 additions & 40 deletions package/MDAnalysis/lib/log.py
Original file line number Diff line number Diff line change
Expand Up @@ -86,22 +86,35 @@
"""
import sys
import logging
import re
import os

from tqdm.auto import tqdm

# from MDAnalysis.lib.util import deprecate

from .. import version


def start_logging(logfile="MDAnalysis.log", version=version.__version__):
def start_logging(stream="MDAnalysis.log", version=version.__version__):
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Under semantic versioning, you need to maintain backwards compatibility. So something like

def start_logging(stream="MDAnalysis.log", logfile=None, version=version.__version__):
  if logfile is not None:
     warnings.warn(DeprecationWarning, "logfile kwarg will be removed in MDAnalysis 3.0.0, use stream instead"
     # we probably have a dedicated function for the deprecation warning...
     stream = logfile
  ...

"""Start logging of messages to file and console.

The default logfile is named `MDAnalysis.log` and messages are
logged with the tag *MDAnalysis*.
"""
create("MDAnalysis", logfile=logfile)
create(
"MDAnalysis",
stream=stream,
level="DEBUG",
fmt="%(asctime)s %(name)-12s %(levelname)-8s %(message)s",
)
create(
"MDAnalysis",
stream=sys.stdout,
level="INFO",
fmt="%(name)-12s: %(levelname)-8s %(message)s",
)
Comment on lines +104 to +115
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

All of this is intend to replicate old behavior from the original.

logging.getLogger("MDAnalysis").info(
Comment thread
jauy123 marked this conversation as resolved.
"MDAnalysis %s STARTED logging to %r", version, logfile
f"MDAnalysis {version} STARTED logging to {stream!r}"
)


Expand All @@ -112,7 +125,13 @@ def stop_logging():
clear_handlers(logger) # this _should_ do the job...


Copy link
Copy Markdown
Contributor Author

@jauy123 jauy123 Apr 23, 2026

Choose a reason for hiding this comment

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

In general, I guess this PR make create() a more general type of function which add handlers to a master logger, so should it get renamed?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Probably yes to renaming, but you'd need to

  1. deprecate the old function
  2. schedule for removal in 3.0.0

because we're following semantic versioning https://semver.org and we cannot remove documented functionality in a public function until we make a new major release.

Similarly, we cannot just rename kwargs in a public function like create(). You have to keep the old one around and deprecated them.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Probably should raise an issue of it as well

def create(logger_name="MDAnalysis", logfile="MDAnalysis.log"):
def create(
logger_name="MDAnalysis",
stream="MDAnalysis.log",
level="DEBUG",
fmt=None,
mode="a",
):
"""Create a top level logger.

- The file logger logs everything (including DEBUG).
Expand All @@ -132,23 +151,20 @@ def create(logger_name="MDAnalysis", logfile="MDAnalysis.log"):

logger = logging.getLogger(logger_name)

logger.setLevel(logging.DEBUG)

# handler that writes to logfile
logfile_handler = logging.FileHandler(logfile)
logfile_formatter = logging.Formatter(
"%(asctime)s %(name)-12s %(levelname)-8s %(message)s"
)
logfile_handler.setFormatter(logfile_formatter)
logger.addHandler(logfile_handler)

# define a Handler which writes INFO messages or higher to the sys.stderr
console_handler = logging.StreamHandler()
console_handler.setLevel(logging.INFO)
# set a format which is simpler for console use
formatter = logging.Formatter("%(name)-12s: %(levelname)-8s %(message)s")
console_handler.setFormatter(formatter)
logger.addHandler(console_handler)
# This checks for file-like object per duck typing
# https://docs.python.org/3/library/logging.handlers.html#streamhandler
if hasattr(stream, "write") and hasattr(stream, "flush"):
handler = logging.StreamHandler(stream)
elif isinstance(stream, (str, os.PathLike)):
handler = logging.FileHandler(stream, mode=mode)
else:
raise TypeError(
"Input Stream is neither a string, PathLike object or a stream"
)

handler.setLevel(level.upper())
handler.setFormatter(logging.Formatter(fmt))
logger.addHandler(handler)

return logger

Expand All @@ -159,27 +175,10 @@ def clear_handlers(logger):
(only important for reload/debug cycles...)

"""
for h in logger.handlers:
for h in list(logger.handlers):
logger.removeHandler(h)
Comment on lines +178 to 179
Copy link
Copy Markdown
Contributor Author

@jauy123 jauy123 Apr 24, 2026

Choose a reason for hiding this comment

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

There's a really subtle bug with the original version. logger.removeHandler(h) mutates the original logger.handlers so it "skips" handlers in the list. Putting in a list creates a copy, so the entire elements get deleted

See test_stop_logging under TestConvenienceFunctions. The original version without list would have failed that test.



class NullHandler(logging.Handler):
"""Silent Handler.

Useful as a default::

h = NullHandler()
logging.getLogger("MDAnalysis").addHandler(h)
del h

see the advice on logging and libraries in
http://docs.python.org/library/logging.html?#configuring-logging-for-a-library
"""

def emit(self, record):
pass


Comment on lines -166 to -182
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

See #5368

class ProgressBar(tqdm):
r"""Display a visual progress bar and time estimate.

Expand Down
35 changes: 33 additions & 2 deletions testsuite/MDAnalysisTests/lib/test_log.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,43 @@
# MDAnalysis: A Toolkit for the Analysis of Molecular Dynamics Simulations.
# J. Comput. Chem. 32 (2011), 2319--2327, doi:10.1002/jcc.21787
#
import warnings
import pytest

import sys
import logging
import MDAnalysis as mda

from os.path import basename
from MDAnalysis.lib.log import ProgressBar


class TestConvenienceFunctions:
def test_start_logging(self, tmp_path):
mda.start_logging(tmp_path / "MDAnalysis.log")
logger = logging.getLogger("MDAnalysis")

# Test expected handlers' presence and behavior
assert any(isinstance(h, logging.NullHandler) for h in logger.handlers)
any(
isinstance(h, logging.FileHandler)
and basename(h.stream.name) == "MDAnalysis.log"
and h.level == logging.DEBUG
for h in logger.handlers
)
assert any(
isinstance(h, logging.StreamHandler)
and h.stream is sys.stdout
and h.level == logging.INFO
for h in logger.handlers
)

def test_stop_logging(self, tmp_path):
mda.lib.log.start_logging(tmp_path / "MDAnalysis.log")
logger = logging.getLogger("MDAnalysis")
mda.lib.log.stop_logging()

assert len(logger.handlers) == 0


class TestProgressBar(object):

def test_output(self, capsys):
Expand Down
75 changes: 0 additions & 75 deletions testsuite/MDAnalysisTests/utils/test_log.py

This file was deleted.

Loading