Skip to content

Fix import failure when warnings.showwarning is overridden (e.g. under pytest-cov)#20019

Open
Mohit-Ak wants to merge 1 commit into
astropy:mainfrom
Mohit-Ak:fix/logger-tolerate-overridden-showwarning
Open

Fix import failure when warnings.showwarning is overridden (e.g. under pytest-cov)#20019
Mohit-Ak wants to merge 1 commit into
astropy:mainfrom
Mohit-Ak:fix/logger-tolerate-overridden-showwarning

Conversation

@Mohit-Ak

@Mohit-Ak Mohit-Ak commented Jul 3, 2026

Copy link
Copy Markdown

Description

Fixes #20005.

import astropy can fail outright with a LoggingError when another library has
replaced warnings.showwarning (or sys.excepthook) after Astropy installed its
own handler. This bites pytest-cov users in particular — the coverage plugin
installs its own showwarning, and then any package that imports Astropy at
collection time (the issue came in from astroquery.mast) crashes on import:

astropy/logger.py:498: in _set_defaults
    self.disable_warnings_logging()
astropy/logger.py:278: in disable_warnings_logging
    raise LoggingError(
E   astropy.logger.LoggingError: Cannot disable warnings logging: warnings.showwarning
    was not set by this logger, or has been overridden

Root cause. _init_log() runs at import time and calls
AstropyLogger._set_defaults(), which resets any previously installed hooks by
calling disable_warnings_logging() / disable_exception_logging(). Those
methods deliberately raise LoggingError when warnings.showwarning /
sys.excepthook no longer points at Astropy's handler — a sensible contract for a
direct caller, but fatal here, because _set_defaults() is a best-effort reset
that runs unconditionally during import astropy.

Fix. Make the import-time reset tolerant: _set_defaults() now catches the
LoggingError from the two disable calls, leaves the third-party hook in place
(it never clobbers someone else's showwarning/excepthook), and just clears
Astropy's own bookkeeping so the logger returns to a clean state. The public
disable_warnings_logging() / disable_exception_logging() API is unchanged —
they still raise for direct callers, so the existing contract tests
(test_warnings_logging_overridden, test_exception_logging_overridden) still
pass. Because conf.log_warnings defaults to True, _set_defaults() then
re-installs Astropy's own handler on top and records the third party's hook as the
recoverable original, so warning capture keeps working and the other library's
hook is restored if Astropy is later disabled.

I fixed both the showwarning path (the reported one) and the sibling
sys.excepthook path in the same method, since they make the identical
assumption.

How was this tested?

Reproduced the exact failure first, then confirmed the fix, on a source build of
main:

import warnings
from astropy import log
from astropy.logger import LoggingError

warnings.showwarning = lambda *a, **k: None   # stand in for pytest-cov et al.
log._set_defaults()                           # RED on main: raises LoggingError
                                              # GREEN after fix: returns cleanly

I also confirmed that with warnings.showwarning overridden before the import,
import astropy now succeeds instead of raising.

Two regression tests were added to astropy/tests/test_logger.py
(test_set_defaults_tolerates_overridden_showwarning and
test_set_defaults_tolerates_overridden_excepthook).

Full logger suite is green and lint is clean at the repo-pinned tool version:

$ python -m pytest astropy/tests/test_logger.py -q
33 passed, 1 skipped

$ ruff check astropy/logger.py astropy/tests/test_logger.py   # ruff 0.15.15
All checks passed!
$ ruff format --check astropy/logger.py astropy/tests/test_logger.py
2 files already formatted

A changelog fragment is included at docs/changes/20005.bugfix.rst (named after
the issue; happy to rename it to the PR number if preferred).

astropy.logger._set_defaults() runs at import time via _init_log(). It
unconditionally called disable_warnings_logging()/disable_exception_logging()
to reset previously installed hooks, but those methods raise LoggingError when
warnings.showwarning / sys.excepthook has been replaced by another library
after Astropy installed its own (e.g. the pytest-cov coverage plugin). That
made 'import astropy' fail outright under such tools.

Make the import-time reset best-effort: catch LoggingError from the disable
calls and relinquish Astropy's own hook bookkeeping while leaving the
third-party hook untouched. The public disable_*_logging() API contract is
unchanged (they still raise for direct callers).

Adds regression tests for both the warnings and exception hook paths.

Fixes astropy#20005
@github-actions

github-actions Bot commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Thank you for your contribution to Astropy! 🌌 This checklist is meant to remind the package maintainers who will review this pull request of some common things to look for.

  • Do the proposed changes actually accomplish desired goals?
  • Do the proposed changes follow the Astropy coding guidelines?
  • Are tests added/updated as required? If so, do they follow the Astropy testing guidelines?
  • Are docs added/updated as required? If so, do they follow the Astropy documentation guidelines?
  • Is rebase and/or squash necessary? If so, please provide the author with appropriate instructions. Also see instructions for rebase and squash.
  • Did the CI pass? If no, are the failures related? If you need to run daily and weekly cron jobs as part of the PR, please apply the "Extra CI" label. Codestyle issues can be fixed by the bot.
  • Is a change log needed? If yes, did the change log check pass? If no, add the "no-changelog-entry-needed" label. If this is a manual backport, use the "skip-changelog-checks" label unless special changelog handling is necessary.
  • Is this a big PR that makes a "What's new?" entry worthwhile and if so, is (1) a "what's new" entry included in this PR and (2) the "whatsnew-needed" label applied?
  • At the time of adding the milestone, if the milestone set requires a backport to release branch(es), apply the appropriate "backport-X.Y.x" label(s) before merge.

@Mohit-Ak Mohit-Ak marked this pull request as ready for review July 3, 2026 19:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Astropy logger does not allow pytest-cov plugin

1 participant