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

Replaced __internal__ argument with module attribute #15

Closed

Conversation

radarhere
Copy link

Suggestion for #6381

Rather than adding an __internal__ argument to method definitions, this PR adds and removes a _deprecate._suppress_internal_deprecations setting.

This may or may not be preferable. See what you think.

@nulano
Copy link
Owner

nulano commented Jun 27, 2022

While I'm not against the idea, I would worry about race conditions causing the _suppress_internal_deprecations flag to be cleared prematurely raising unexpected warnings. To avoid them, I think it would have to be a thread-local variable, which seems to me like an unnecessarily complicated solution.

@radarhere
Copy link
Author

@hugovk did you have a preference here?

@hugovk
Copy link

hugovk commented Jul 1, 2022

Is the idea to prevent deprecation warnings being raised when we're calling deprecated code internally?

Ideally we shouldn't call deprecated code internally, because it's going to go away, but in the short term, can we catch the warning when called internally with something like warnings.catch_warnings?

@nulano
Copy link
Owner

nulano commented Jul 1, 2022

The ImageDraw.textsize function is just a wrapper for *Font.getsize, so it is expected that deprecated code is called internally until both are removed.

I wasn't aware of warnings.catch_warnings, but considered implementing a similar context manager.
However, it is also not thread-safe:

https://docs.python.org/3/library/warnings.html#temporarily-suppressing-warnings (emphasis mine)

While within the context manager all warnings will simply be ignored. This allows you to use known-deprecated code without having to see the warning while not suppressing the warning for other code that might not be aware of its use of deprecated code. Note: this can only be guaranteed in a single-threaded application. If two or more threads use the catch_warnings context manager at the same time, the behavior is undefined.

@radarhere
Copy link
Author

Ok, I've created #19 instead.

@radarhere radarhere closed this Jul 1, 2022
@radarhere radarhere deleted the deprecate-getsize branch July 19, 2022 00:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants