gh-129027: Raise DeprecationWarning for sys._clear_type_cache#129043
Conversation
tomasr8
left a comment
There was a problem hiding this comment.
sys._clear_type_cache is used in other tests as well which will now raise a DeprecationWarning. I think we should suppress the warnings in the relevant tests (or maybe switch to sys._clear_internal_caches()? though that might make backporting tests to 3.12 more difficult since the function was added in 3.13)
|
This will also require a news entry :) |
|
@tomasr8 Thanks 👍🏾 . Please review again? |
|
The What's New is still missing (it's something that you need to add in Doc/whatsnew/3.14.rst I think). The NEWS (blurb) entry is only for the full changelog, but deprecation notices are also communicated there so that they have better visibility. You can put this change in the Pending Removal section (as we are now emitting a warning) |
|
I'll review it in a few minutes, but I have some comments on some wording. And I think we already mentioned to avoid force pushing, so please do so in the future. Avoid rebasing to merge upstream and prefer using the update button instead. Or if it becomes too messy, just create a new branch to be sure you don't trigger unnecessary review requests |
picnixz
left a comment
There was a problem hiding this comment.
After commenting the files, I think you can create a wrapper around _clear_type_cache that suppresses its deprecation warning use instead of suppressing the warning around the entire test. If other deprecation warnings appear we want them to be printed.
| type_assign_specific_version_unsafe = _testinternalcapi.type_assign_specific_version_unsafe | ||
| type_assign_version = _testcapi.type_assign_version | ||
| type_modified = _testcapi.type_modified | ||
| ignore_deprecation = warnings_helper.ignore_warnings(category=DeprecationWarning) |
There was a problem hiding this comment.
This should be a dummy decorator if _clear_type_cache is None.
| self.assertEqual(rc, 0) | ||
|
|
||
| @unittest.skipUnless(sysconfig.get_config_var('Py_TRACE_REFS'), "Requires --with-trace-refs build option") | ||
| @warnings_helper.ignore_warnings(category=DeprecationWarning) |
There was a problem hiding this comment.
Can we use this one only when doing assert_python_ok?
| self.assertIn(b'True', out) | ||
|
|
||
| @unittest.skipUnless(sysconfig.get_config_var('Py_TRACE_REFS'), "Requires --with-trace-refs build option") | ||
| @warnings_helper.ignore_warnings(category=DeprecationWarning) |
| self.assertNotEqual(type_get_version(C), 0) | ||
| self.assertNotEqual(type_get_version(C), c_ver) | ||
|
|
||
| @ignore_deprecation |
There was a problem hiding this comment.
Can we only protect the _clear_type_cache call instead of the entire test?
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
…0vxzZ.rst Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
|
@picnixz Please review again. |
| with warnings.catch_warnings(): | ||
| warnings.simplefilter("ignore", DeprecationWarning) |
There was a problem hiding this comment.
Maybe you can wrap clear_type_cache with a waraning filter:
def clear_type_cache():
with warnings.catch_warnings():
...
_clear_type_cache()|
@picnixz Done. |
|
@srinivasreddy There are some conflicts with main now, could you fix them please? |
|
@tomasr8 Done. |
DeprecationWarningforsys._clear_type_cache#129027