From 2e82f1556a018b609cb53f94a15169aa69e3e99a Mon Sep 17 00:00:00 2001 From: Louis Maddox Date: Fri, 18 Aug 2023 21:21:04 +0100 Subject: [PATCH 1/7] fix: prevent potential memory leak --- pandas/util/_exceptions.py | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/pandas/util/_exceptions.py b/pandas/util/_exceptions.py index 573f76a63459b..cf0b836a1972c 100644 --- a/pandas/util/_exceptions.py +++ b/pandas/util/_exceptions.py @@ -43,14 +43,17 @@ def find_stack_level() -> int: # https://stackoverflow.com/questions/17407119/python-inspect-stack-is-slow frame = inspect.currentframe() - n = 0 - while frame: - fname = inspect.getfile(frame) - if fname.startswith(pkg_dir) and not fname.startswith(test_dir): - frame = frame.f_back - n += 1 - else: - break + try: + n = 0 + while frame: + fname = inspect.getfile(frame) + if fname.startswith(pkg_dir) and not fname.startswith(test_dir): + frame = frame.f_back + n += 1 + else: + break + finally: + del frame # Prevent potential memory leak return n From 89689be72b3daf516d00b83e1c686d1be85a6e54 Mon Sep 17 00:00:00 2001 From: Louis Maddox Date: Fri, 18 Aug 2023 22:05:05 +0100 Subject: [PATCH 2/7] test: unit test to cover find_stack_level utility function --- pandas/tests/util/test_find_stack_level.py | 22 ++++++++++++++++++++++ pandas/util/_exceptions.py | 9 +++++---- 2 files changed, 27 insertions(+), 4 deletions(-) create mode 100644 pandas/tests/util/test_find_stack_level.py diff --git a/pandas/tests/util/test_find_stack_level.py b/pandas/tests/util/test_find_stack_level.py new file mode 100644 index 0000000000000..26144faa0d527 --- /dev/null +++ b/pandas/tests/util/test_find_stack_level.py @@ -0,0 +1,22 @@ +from pytest import ( + fixture, + mark, +) + +from pandas.util._exceptions import find_stack_level + + +@mark.parametrize("above,below", [(0, 1)]) +def test_find_stack_level(above, below): + """ + Note that this test would not be expected to pass on CPython implementations which + don't support getting the frame with currentframe (which would always return None). + """ + + def test_stack_level() -> int: + return find_stack_level() + + top_lvl = find_stack_level() + assert top_lvl == above, f"Expected stack level {above} but got {top_lvl}" + sub_lvl = nested_call() + assert sub_lvl == nest_lvl, f"Expected stack level {below} but got {sub_lvl}" diff --git a/pandas/util/_exceptions.py b/pandas/util/_exceptions.py index cf0b836a1972c..0aed3409a5169 100644 --- a/pandas/util/_exceptions.py +++ b/pandas/util/_exceptions.py @@ -4,7 +4,8 @@ import inspect import os import re -from typing import TYPE_CHECKING +from types import FrameType +from typing import TYPE_CHECKING, Optional import warnings if TYPE_CHECKING: @@ -42,12 +43,12 @@ def find_stack_level() -> int: test_dir = os.path.join(pkg_dir, "tests") # https://stackoverflow.com/questions/17407119/python-inspect-stack-is-slow - frame = inspect.currentframe() + frame: Optional[FrameType] = inspect.currentframe() try: n = 0 while frame: - fname = inspect.getfile(frame) - if fname.startswith(pkg_dir) and not fname.startswith(test_dir): + filename = inspect.getfile(frame) + if filename.startswith(pkg_dir) and not filename.startswith(test_dir): frame = frame.f_back n += 1 else: From ea712abb57b5990bfa81ccc7fb551cd3d28b51b7 Mon Sep 17 00:00:00 2001 From: Louis Maddox Date: Fri, 18 Aug 2023 22:14:01 +0100 Subject: [PATCH 3/7] style: linting fixes and forgot to rename a funcdef --- pandas/tests/util/test_find_stack_level.py | 8 +++----- pandas/util/_exceptions.py | 9 ++++++--- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/pandas/tests/util/test_find_stack_level.py b/pandas/tests/util/test_find_stack_level.py index 26144faa0d527..ded035f153181 100644 --- a/pandas/tests/util/test_find_stack_level.py +++ b/pandas/tests/util/test_find_stack_level.py @@ -1,7 +1,4 @@ -from pytest import ( - fixture, - mark, -) +from pytest import mark from pandas.util._exceptions import find_stack_level @@ -13,10 +10,11 @@ def test_find_stack_level(above, below): don't support getting the frame with currentframe (which would always return None). """ - def test_stack_level() -> int: + def nested_call() -> int: return find_stack_level() top_lvl = find_stack_level() assert top_lvl == above, f"Expected stack level {above} but got {top_lvl}" + sub_lvl = nested_call() assert sub_lvl == nest_lvl, f"Expected stack level {below} but got {sub_lvl}" diff --git a/pandas/util/_exceptions.py b/pandas/util/_exceptions.py index 0aed3409a5169..2c4090290c910 100644 --- a/pandas/util/_exceptions.py +++ b/pandas/util/_exceptions.py @@ -4,12 +4,15 @@ import inspect import os import re -from types import FrameType -from typing import TYPE_CHECKING, Optional +from typing import ( + TYPE_CHECKING, + Optional, +) import warnings if TYPE_CHECKING: from collections.abc import Generator + from types import FrameType @contextlib.contextmanager @@ -43,7 +46,7 @@ def find_stack_level() -> int: test_dir = os.path.join(pkg_dir, "tests") # https://stackoverflow.com/questions/17407119/python-inspect-stack-is-slow - frame: Optional[FrameType] = inspect.currentframe() + frame: FrameType | None = inspect.currentframe() try: n = 0 while frame: From 0c3f5924379949dd84d49c18dd89b47372ee2127 Mon Sep 17 00:00:00 2001 From: Louis Maddox Date: Fri, 18 Aug 2023 22:16:10 +0100 Subject: [PATCH 4/7] style: linter fix and forgot to rename a variable in test --- pandas/tests/util/test_find_stack_level.py | 2 +- pandas/util/_exceptions.py | 5 +---- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/pandas/tests/util/test_find_stack_level.py b/pandas/tests/util/test_find_stack_level.py index ded035f153181..3f1969186c66b 100644 --- a/pandas/tests/util/test_find_stack_level.py +++ b/pandas/tests/util/test_find_stack_level.py @@ -17,4 +17,4 @@ def nested_call() -> int: assert top_lvl == above, f"Expected stack level {above} but got {top_lvl}" sub_lvl = nested_call() - assert sub_lvl == nest_lvl, f"Expected stack level {below} but got {sub_lvl}" + assert sub_lvl == below, f"Expected stack level {below} but got {sub_lvl}" diff --git a/pandas/util/_exceptions.py b/pandas/util/_exceptions.py index 2c4090290c910..cfe937e45ba02 100644 --- a/pandas/util/_exceptions.py +++ b/pandas/util/_exceptions.py @@ -4,10 +4,7 @@ import inspect import os import re -from typing import ( - TYPE_CHECKING, - Optional, -) +from typing import TYPE_CHECKING import warnings if TYPE_CHECKING: From 3ac7e2684b411378904d125730c5e7dc94509768 Mon Sep 17 00:00:00 2001 From: Louis Maddox Date: Fri, 18 Aug 2023 22:39:41 +0100 Subject: [PATCH 5/7] test: simplify test and correct value (direct call stack level is 1 not 0) --- pandas/tests/util/test_find_stack_level.py | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/pandas/tests/util/test_find_stack_level.py b/pandas/tests/util/test_find_stack_level.py index 3f1969186c66b..a5e3dde3ddfae 100644 --- a/pandas/tests/util/test_find_stack_level.py +++ b/pandas/tests/util/test_find_stack_level.py @@ -3,18 +3,12 @@ from pandas.util._exceptions import find_stack_level -@mark.parametrize("above,below", [(0, 1)]) -def test_find_stack_level(above, below): +@mark.parametrize("expected", [1]) +def test_find_stack_level(expected): """ Note that this test would not be expected to pass on CPython implementations which don't support getting the frame with currentframe (which would always return None). """ - def nested_call() -> int: - return find_stack_level() - top_lvl = find_stack_level() - assert top_lvl == above, f"Expected stack level {above} but got {top_lvl}" - - sub_lvl = nested_call() - assert sub_lvl == below, f"Expected stack level {below} but got {sub_lvl}" + assert top_lvl == expected, f"Expected stack level {expected} but got {top_lvl}" From 2a39d23b64ceae4b67c62595ed733081f6ebac63 Mon Sep 17 00:00:00 2001 From: Matthew Roeschke <10647082+mroeschke@users.noreply.github.com> Date: Thu, 25 Jan 2024 17:21:26 -0800 Subject: [PATCH 6/7] Add pydoc reference, remove file --- pandas/_testing/_warnings.py | 7 ++++++- pandas/tests/util/test_find_stack_level.py | 14 -------------- pandas/util/_exceptions.py | 4 +++- 3 files changed, 9 insertions(+), 16 deletions(-) delete mode 100644 pandas/tests/util/test_find_stack_level.py diff --git a/pandas/_testing/_warnings.py b/pandas/_testing/_warnings.py index 731a6df776c4c..f5787129433ef 100644 --- a/pandas/_testing/_warnings.py +++ b/pandas/_testing/_warnings.py @@ -226,4 +226,9 @@ def _assert_raised_with_correct_stacklevel( f"File where warning is raised: {actual_warning.filename} != " f"{caller_filename}. Warning message: {actual_warning.message}" ) - assert actual_warning.filename == caller_filename, msg + try: + assert actual_warning.filename == caller_filename, msg + finally: + # See note in + # https://docs.python.org/3/library/inspect.html#inspect.Traceback + del frame diff --git a/pandas/tests/util/test_find_stack_level.py b/pandas/tests/util/test_find_stack_level.py deleted file mode 100644 index a5e3dde3ddfae..0000000000000 --- a/pandas/tests/util/test_find_stack_level.py +++ /dev/null @@ -1,14 +0,0 @@ -from pytest import mark - -from pandas.util._exceptions import find_stack_level - - -@mark.parametrize("expected", [1]) -def test_find_stack_level(expected): - """ - Note that this test would not be expected to pass on CPython implementations which - don't support getting the frame with currentframe (which would always return None). - """ - - top_lvl = find_stack_level() - assert top_lvl == expected, f"Expected stack level {expected} but got {top_lvl}" diff --git a/pandas/util/_exceptions.py b/pandas/util/_exceptions.py index cfe937e45ba02..5f50838d37315 100644 --- a/pandas/util/_exceptions.py +++ b/pandas/util/_exceptions.py @@ -54,7 +54,9 @@ def find_stack_level() -> int: else: break finally: - del frame # Prevent potential memory leak + # See note in + # https://docs.python.org/3/library/inspect.html#inspect.Traceback + del frame return n From c8b048d589ba91dff90ea8823e3541668ce9352c Mon Sep 17 00:00:00 2001 From: Matthew Roeschke <10647082+mroeschke@users.noreply.github.com> Date: Thu, 25 Jan 2024 17:24:12 -0800 Subject: [PATCH 7/7] Call finally closer to its use --- pandas/_testing/_warnings.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/pandas/_testing/_warnings.py b/pandas/_testing/_warnings.py index f5787129433ef..cff28f6a20472 100644 --- a/pandas/_testing/_warnings.py +++ b/pandas/_testing/_warnings.py @@ -220,15 +220,15 @@ def _assert_raised_with_correct_stacklevel( frame = inspect.currentframe() for _ in range(4): frame = frame.f_back # type: ignore[union-attr] - caller_filename = inspect.getfile(frame) # type: ignore[arg-type] - msg = ( - "Warning not set with correct stacklevel. " - f"File where warning is raised: {actual_warning.filename} != " - f"{caller_filename}. Warning message: {actual_warning.message}" - ) try: - assert actual_warning.filename == caller_filename, msg + caller_filename = inspect.getfile(frame) # type: ignore[arg-type] finally: # See note in # https://docs.python.org/3/library/inspect.html#inspect.Traceback del frame + msg = ( + "Warning not set with correct stacklevel. " + f"File where warning is raised: {actual_warning.filename} != " + f"{caller_filename}. Warning message: {actual_warning.message}" + ) + assert actual_warning.filename == caller_filename, msg