Skip to content
Draft
Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
33a8005
intial pseudocode
jauy123 Apr 23, 2026
1bf0ee1
inital thought code (hadn't run it yet)
jauy123 Apr 23, 2026
24a6c31
more comments
jauy123 Apr 23, 2026
7a5c0b3
general cleanup
jauy123 Apr 23, 2026
f1be0df
Post black
jauy123 Apr 23, 2026
dbcfb1c
removed loop based control
jauy123 Apr 23, 2026
93966fb
Added formatter to handler
jauy123 Apr 23, 2026
c5f6133
Added format to start_logging() to replicate old behavior
jauy123 Apr 23, 2026
e30d0ab
Applied flake8
jauy123 Apr 23, 2026
5a5906d
notes for test later
jauy123 Apr 23, 2026
f679251
fix linter
jauy123 Apr 23, 2026
2ba1c2e
put logger back to start_logging()
jauy123 Apr 24, 2026
cca9258
cleaned up create()
jauy123 Apr 24, 2026
94f056f
removed MDAnalysis NullHandler
jauy123 Apr 24, 2026
7f5812f
readded top level import -- it broke some tests
jauy123 Apr 24, 2026
ba9315e
ported over tests
jauy123 Apr 24, 2026
0e36231
Used black
jauy123 Apr 24, 2026
e63f5ef
Added Tests for start_logging() and stop_logging()
jauy123 Apr 24, 2026
a81e4ad
Added comment
jauy123 Apr 24, 2026
f25aec7
Added comment
jauy123 Apr 24, 2026
bdaf241
post black
jauy123 Apr 24, 2026
ab00172
Merge branch 'logging' of github.com:jauy123/mdanalysis into logging
jauy123 Apr 24, 2026
d0b732f
Added comments about deprecations
jauy123 Apr 24, 2026
ec0ed25
Replaced logging.getLogger(MDAnalysis) with logging.getLogger(__name__)
jauy123 Apr 24, 2026
6d1a675
refined comments
jauy123 Apr 24, 2026
74bcc03
Updated tests
jauy123 Apr 24, 2026
0fdd02e
Change __name__ to MDAnalysis fix tests
jauy123 Apr 24, 2026
d952902
Updated Tests
jauy123 Apr 24, 2026
c34eca5
Added future tests
jauy123 Apr 24, 2026
f9a4961
Finished tests for mda.start_logging() and mda.stop_logging()
jauy123 May 1, 2026
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
  ...

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 my comment above in line 101. Why would a user want to manually change the version in this function? The version keyword should be deprecated and removed.

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.

That's fair. (I think this was a pattern where we wanted to avoid accessing global scope inside functions and passed the values through kwargs.)

"""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 +110 to +121
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.

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 @@ -131,24 +150,21 @@ 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)
logger.setLevel(level.upper())

# 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.setFormatter(logging.Formatter(fmt))
logger.addHandler(handler)

return logger

Expand All @@ -163,23 +179,6 @@ def clear_handlers(logger):
logger.removeHandler(h)
Comment on lines +203 to 204
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.

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.

yikes :-) – good catch



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

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.

Should deprecated instead of remove huh

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.

unfortunately, yes

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

Expand Down
16 changes: 14 additions & 2 deletions testsuite/MDAnalysisTests/lib/test_log.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,24 @@
# 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 logging

import MDAnalysis
from MDAnalysis.lib.log import ProgressBar


def test_start_stop_logging():
try:
MDAnalysis.log.start_logging()
logger = logging.getLogger("MDAnalysis")
logger.info("Using the MDAnalysis logger works")
except Exception as err:
raise AssertionError("Problem with logger: {0}".format(err))
finally:
MDAnalysis.log.stop_logging()


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