Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

filterwarnings causes warnings.warn to raise TypeError on non-string messages #103577

Open
zanieb opened this issue Apr 16, 2023 · 7 comments
Open
Labels
stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@zanieb
Copy link

zanieb commented Apr 16, 2023

Bug report

When using filterwarnings, a TypeError is raised when a non-string value is passed to warnings.warn which differs from the default behavior of warnings.warn.

import warnings

# Does not raise a type error
warnings.warn(1)

# Raises a type error
with warnings.catch_warnings():
    warnings.filterwarnings("ignore", "test")
    warnings.warn(1)

The traceback does not demonstrate that this is a detail of warning filtering:

/Users/mz/eng/src/madkinsz/cpython-103577/example.py:4: UserWarning: 1
  warnings.warn(1)
Traceback (most recent call last):
  File "/Users/mz/eng/src/madkinsz/cpython-103577/example.py", line 9, in <module>
    warnings.warn(1)
TypeError: expected string or bytes-like object

Filtering warnings should not change the accepted types of warnings.warn.

Reproduction example at https://github.com/madkinsz/cpython-103577
Originally raised at pytest-dev/pytest#10865

Your environment

Reproduced Ubuntu with Python 3.7-3.12: https://github.com/madkinsz/cpython-103577/actions/runs/4714383423/jobs/8360711980

Also reproduced manually on macOS with Python 3.10/3.11

@zanieb zanieb added the type-bug An unexpected behavior, bug, or error label Apr 16, 2023
@arhadthedev arhadthedev added the stdlib Python modules in the Lib dir label Apr 16, 2023
@zanieb
Copy link
Author

zanieb commented Apr 16, 2023

I'm happy to help contribute a fix if ya'll agree this behavior isn't ideal :)

@sobolevn
Copy link
Member

sobolevn commented Apr 16, 2023

Are there any real-life use-cases where using warnings.warn with non-str messages make sense?
I've only seen str and Warning subclasses.

Docs also say the same: https://docs.python.org/3/library/warnings.html#warnings.warn
Typeshed also annotates message as either str or Warning: https://github.com/python/typeshed/blob/7dcec5b3db887f518114486b3e4783dde244186d/stdlib/_warnings.pyi#L7-L10

In you case you can use:

>>> with warnings.catch_warnings():
...     warnings.filterwarnings("ignore", "test")
...     warnings.warn(Warning(1))
... 
<stdin>:3: Warning: 1

I don't think that allowing more types in is a good thing.

@zanieb
Copy link
Author

zanieb commented Apr 16, 2023

Hey @sobolevn — I agree that it doesn't make sense to allow more types but it is very surprising for the behavior to change when you enable filtering. This means that you can have a library that works totally fine until warning filtering is used then suddenly a type error will be thrown.

In practice, I encountered this working on prefect where we display a warning based on an exception:

https://github.com/PrefectHQ/prefect/blob/b6d0433a15b8172c0f897a6eac2b4d747350fd1f/src/prefect/logging/handlers.py#L123

Before we added the str cast, the code worked fine in some environments but failed in others.

@sobolevn
Copy link
Member

sobolevn commented Apr 17, 2023

This means that you can have a library that works totally fine until warning filtering is used then suddenly a type error will be thrown.

Yes, this is an eventual side-effect of dynamic typing. It all works untill it does not :)

Typechecker should catch errors like this. I don't think that anything should be done in this case from CPython's side.

Do you agree? Maybe you have ideas of better docs wordings?

@zanieb
Copy link
Author

zanieb commented Apr 17, 2023

I hesitantly agree :) it's a sneaky problem since the error message / traceback does not include any context about filtering. Perhaps:

  1. Add a warning to the documentation of filterwarnings about this case
  2. Improve the traceback i.e. by moving the filtering to a separate utility function so there's a frame indicating that it's related to filtering
  3. Throw a warning when warnings.warn is called with an invalid type (backwards compatible and helpful but not very Pythonic)

@Zac-HD
Copy link
Contributor

Zac-HD commented Jun 18, 2023

IMO we should change from "errors if you called filterwarnings()" to "always errors".

@zanieb
Copy link
Author

zanieb commented Aug 22, 2023

@Zac-HD I'm happy with that too — seems like a breaking change but a reasonable one

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

No branches or pull requests

4 participants