Skip to content

standardize logging.getLogger() to use __name__ (Issue #5369)#5376

Open
Jatin-Khiyani wants to merge 2 commits into
MDAnalysis:developfrom
Jatin-Khiyani:issue-5369
Open

standardize logging.getLogger() to use __name__ (Issue #5369)#5376
Jatin-Khiyani wants to merge 2 commits into
MDAnalysis:developfrom
Jatin-Khiyani:issue-5369

Conversation

@Jatin-Khiyani

@Jatin-Khiyani Jatin-Khiyani commented May 5, 2026

Copy link
Copy Markdown

Fixes #5369

Changes made in this Pull Request

LLM / AI generated code disclosure

LLMs or other AI-powered tools (beyond simple IDE use cases) were used
in this contribution: yes

Used to understand the codebase and the existing logging patterns.
All code changes were made manually following the guidance in the issue.

PR Checklist

  • Issue raised/referenced?
  • Tests updated/added?
  • Documentation updated/added?
  • package/CHANGELOG file updated?
  • Is your name in package/AUTHORS?
  • LLM/AI disclosure was updated.

Developers Certificate of Origin

I certify that I can submit this code contribution as described in the
Developer Certificate of Origin,
under the MDAnalysis LICENSE.


📚 Documentation preview 📚: https://mdanalysis--5376.org.readthedocs.build/en/5376/

Replace hardcoded logger name strings with __name__ across
analysis, coordinates, topology, converters, core and guesser
modules.

Some modules had mismatched names (e.g. coordinates/TRJ.py used
"AMBER", coordinates/PDB.py had typo "PBD") — these are now
corrected as a side effect of this change.
@codecov

codecov Bot commented May 5, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.85%. Comparing base (32b7808) to head (55e378e).

Additional details and impacted files
@@           Coverage Diff            @@
##           develop    #5376   +/-   ##
========================================
  Coverage    93.85%   93.85%           
========================================
  Files          182      182           
  Lines        22509    22509           
  Branches      3202     3202           
========================================
+ Hits         21125    21126    +1     
  Misses         922      922           
+ Partials       462      461    -1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@jauy123

jauy123 commented May 6, 2026

Copy link
Copy Markdown
Contributor

There is only one concern with this change. It does consolidate all the loggers under a single MDAnalysis logger rather than have a separate logger instance per submodule. Would this be desirable behavior?

I would say so since it would follow standard library guidelines. Also, the fact that the none of the existing tests blew up at this change shows that the "separate logger instance" behavior wasn't really considered in the original iteration of the code.

@orbeckst orbeckst added the AI-assisted Generated with significant AI/LLM assistance label Jun 22, 2026

@orbeckst orbeckst left a comment

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.

Overall looking good, please see comments.

Comment thread package/CHANGELOG Outdated
Comment thread package/CHANGELOG Outdated

import logging

logger = logging.getLogger("MDAnalysis.coordinates.GROMOS11")

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.

keep

from . import base
from ..lib import util
from ..lib.util import store_init_arguments
logger = logging.getLogger("MDAnalysis.coordinates.AMBER")

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.

keep

@orbeckst orbeckst left a comment

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.

Please address my remaining changes and add yourself to AUTHORS (as a first-time contributor).

Also check the formatting issues that the linter complains about.

@orbeckst

Copy link
Copy Markdown
Member

The docs build failure is not your fault, I raised #5410 for it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AI-assisted Generated with significant AI/LLM assistance

Projects

None yet

Development

Successfully merging this pull request may close these issues.

logging.getLogger() should follow a standardized syntax.

3 participants