Skip to content

gh-125893: Add type check for category argument in warnings.simplefilter and warnings.filterwarning - #136305

Open
hasrat17 wants to merge 10 commits into
python:mainfrom
hasrat17:fix-warnings-simplefilter-filterwarnings
Open

gh-125893: Add type check for category argument in warnings.simplefilter and warnings.filterwarning#136305
hasrat17 wants to merge 10 commits into
python:mainfrom
hasrat17:fix-warnings-simplefilter-filterwarnings

Conversation

@hasrat17

@hasrat17 hasrat17 commented Jul 4, 2025

Copy link
Copy Markdown

This PR addresses gh-125893 by adding a type check for the category argument in warnings.simplefilter and warnings.filterwarning,

Previously, warnings.filterwarnings correctly raised an error when the category argument was not a class, but warnings.simplefilter accepted invalid types without raising any error. This inconsistency could lead to confusion and improper warning filtering.

Both warnings.simplefilter and warnings.filterwarning were not raising any warnings while ran with python -O but now this is also handled.

$ ./amd64/python.exe -O -c "import warnings; warnings.resetwarnings(); warnings.simplefilter('default', category='Hello');  print(warnings.filters)"
Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "C:\Users\hasra\OneDrive\Documents\opensource\cpython\Lib\warnings.py", line 182, in simplefilter
    raise TypeError("category must be a Warning subclass, "
TypeError: category must be a Warning subclass, not 'str'
./amd64/python.exe -O -c "import warnings; warnings.resetwarnings(); warnings.filterwarnings('ignore', category=Warning); warnings.filterwarnings('ignore', category='Hello'); print(warnings.filters)"
Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "C:\Users\hasra\OneDrive\Documents\opensource\cpython\Lib\warnings.py", line 149, in filterwarnings
    raise TypeError("category must be a Warning subclass, "
TypeError: category must be a Warning subclass, not 'str'

@bedevere-app

bedevere-app Bot commented Jul 4, 2025

Copy link
Copy Markdown

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

@ZeroIntensity ZeroIntensity 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.

Sorry for the delay on the review. Please add a test case to test_warnings. You can refer to the devguide if you're unsure how to do that.

Comment thread Misc/ACKS Outdated
Gennadiy Zlobin
Doug Zongker
Peter Åstrand
Hasrat Ali Arzoo

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.

We try to keep this sorted by last name. You should be closer to the top.

Comment thread Lib/_py_warnings.py
@ZeroIntensity

Copy link
Copy Markdown
Member

Oh, also, please add a news entry.

@python-cla-bot

python-cla-bot Bot commented Jul 10, 2025

Copy link
Copy Markdown

All commit authors signed the Contributor License Agreement.

CLA signed

@hasrat17

Copy link
Copy Markdown
Author

Thanks for your review! @ZeroIntensity I've addressed all the feedback and updated accordingly.

@hasrat17
hasrat17 requested a review from ZeroIntensity July 10, 2025 15:32
@hasrat17

Copy link
Copy Markdown
Author

Hi @ZeroIntensity
I’ve addressed all the review feedback and updated the branch. Could you please take a look at this PR or assign it to the appropriate reviewer? Thank you!

Comment thread Misc/NEWS.d/next/Windows/2025-07-10-19-28-11.gh-issue-125893.lD8Nyd.rst Outdated
Comment thread Lib/_py_warnings.py Outdated
if isinstance(category, type) and issubclass(category, Warning):
return True
if isinstance(category, tuple):
if __debug__:

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.

I'd still consider the isinstance checks above to be expensive. Could you move this __debug__ check to before any calls to isinstance?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Moved __debug__ before any other checks

with self.assertRaises(ValueError):
self.module.simplefilter('ignore', lineno=-1)

def test_invalid_category_types(self):

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.

We probably need to skip this if we're not in debug mode, so add something like @unittest.skipUnless(__debug__, ...).

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Updated accordingly

@hasrat17
hasrat17 requested a review from ZeroIntensity August 13, 2025 19:40
@hasrat17

Copy link
Copy Markdown
Author

Hi @ZeroIntensity
I've addressed the comments. Please review it whenever you have time. Thanks

@github-actions

Copy link
Copy Markdown

This PR is stale because it has been open for 30 days with no activity.

@github-actions github-actions Bot added the stale Stale PR or inactive for long period of time. label Apr 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting review stale Stale PR or inactive for long period of time.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants