From 7f027ac9d3ea8d941620d24662c1ba439a865b9b Mon Sep 17 00:00:00 2001 From: Chaarvi Bansal Date: Tue, 2 Jul 2024 23:41:03 +0100 Subject: [PATCH 01/11] fix ruff errors --- .pre-commit-config.yaml | 66 +++++++++++++++ ...ce_match_arg_in_assert_produces_warning.py | 84 +++++++++++++++++++ 2 files changed, 150 insertions(+) create mode 100755 scripts/enforce_match_arg_in_assert_produces_warning.py diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 2d5cd9e841df3..4f2ce136f15a3 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -301,3 +301,69 @@ repos: language: python files: ^doc/source/whatsnew/v exclude: ^doc/source/whatsnew/v(0|1|2\.0\.0) + - id: enforce-match-arg-in-assert-produces-warning + name: Enforce the usage of match arg + entry: python scripts/enforce_match_arg_in_assert_produces_warning.py + language: python + files: ^pandas/tests + exclude: | + (?x) + ^( + pandas/tests/computation/test_eval.py| + pandas/tests/frame/test_query_eval.py| + pandas/tests/frame/methods/test_drop.py| + pandas/tests/plotting/test_datetimelike.py| + pandas/tests/plotting/frame/test_frame_color.py| + pandas/tests/plotting/test_hist_method.py| + pandas/tests/plotting/test_boxplot_method.py| + pandas/tests/util/test_deprecate_kwarg.py| + pandas/tests/util/test_deprecate.py| + pandas/tests/util/test_assert_produces_warning.py| + pandas/tests/util/test_deprecate_nonkeyword_arguments.py| + pandas/tests/api/test_types.py| + pandas/tests/api/test_api.py| + pandas/tests/strings/test_find_replace.py| + pandas/tests/arithmetic/test_datetime64.py| + pandas/tests/arithmetic/test_timedelta64.py| + pandas/tests/arithmetic/test_period.py| + pandas/tests/tseries/offsets/test_offsets.py| + pandas/tests/indexes/datetimes/methods/test_shift.py| + pandas/tests/extension/test_period.py| + pandas/tests/io/pytables/test_retain_attributes.py| + pandas/tests/scalar/timestamp/methods/test_round.py| + pandas/tests/scalar/timestamp/methods/test_to_pydatetime.py| + pandas/tests/io/test_sql.py| + pandas/tests/reshape/test_pivot.py| + pandas/tests/plotting/frame/test_hist_box_by.py| + pandas/tests/frame/methods/test_reindex_like.py| + pandas/tests/plotting/test_series.py| + pandas/tests/io/parser/test_unsupported.py| + pandas/tests/series/accessors/test_cat_accessor.py| + pandas/tests/test_algos.py| + pandas/tests/indexing/multiindex/test_multiindex.py| + pandas/tests/indexes/multi/test_drop.py| + pandas/tests/plotting/frame/test_frame.py| + pandas/tests/window/test_numba.py| + pandas/tests/reshape/test_cut.py| + pandas/tests/apply/test_str.py| + pandas/tests/indexes/multi/test_indexing.py| + pandas/tests/io/pytables/test_store.py| + pandas/tests/io/pytables/test_put.py| + pandas/tests/io/pytables/test_round_trip.py| + pandas/tests/io/excel/test_readers.py| + pandas/tests/resample/test_datetime_index.py| + pandas/tests/io/parser/test_c_parser_only.py| + pandas/tests/io/test_stata.py| + pandas/tests/plotting/test_misc.py| + pandas/tests/series/methods/test_equals.py| + pandas/tests/frame/test_block_internals.py| + pandas/tests/indexes/multi/test_sorting.py| + pandas/tests/series/methods/test_reindex_like.py| + pandas/tests/extension/test_sparse.py| + pandas/tests/indexes/test_common.py| + pandas/tests/indexes/datetimes/methods/test_round.py| + pandas/tests/indexing/multiindex/test_loc.py| + pandas/tests/frame/indexing/test_insert.py| + pandas/tests/groupby/test_groupby.py| + )$ + types: [python] diff --git a/scripts/enforce_match_arg_in_assert_produces_warning.py b/scripts/enforce_match_arg_in_assert_produces_warning.py new file mode 100755 index 0000000000000..0c012bfb67d22 --- /dev/null +++ b/scripts/enforce_match_arg_in_assert_produces_warning.py @@ -0,0 +1,84 @@ +# 30th June - spent 1hr 20min +# 1st July - spent close to 1hr 30min + +""" + +Enforce that all usages of tms.assert_produces_warning use +the "match" argument. This will help ensure that users always see +the correct warning message. + +tms.assert_produces_warning(None) is excluded as no warning is +produced. + +""" + + +import argparse +import ast +from collections.abc import Sequence +import sys + +ERROR_MESSAGE = ( + "{path}:{lineno}:{col_offset}: " + '"match" argument missing in tm.assert_produces_warning' + "\n" +) + + +class MatchArgForWarningsChecker(ast.NodeVisitor): + def __init__(self) -> None: + self.error_set = [] + + def visit_Call(self, node) -> None: + if ( isinstance(node.func, ast.Attribute) and + node.func.attr == "assert_produces_warning"): + # only check for attribute function of class/module tm + if ( isinstance(node.func.value, ast.Name) and + node.func.value.id == "tm" ): + # ignore tms.assert_produces_warning(None)/tms.assert_produces_warning() + if ( len(node.args) == 0 or + (isinstance(node.args[0], ast.Constant) and + node.args[0].value is None) ): + return + if not any(keyword.arg == "match" for keyword in node.keywords): + self.error_set.append((node.lineno, node.col_offset)) + + +# Returns true if a file fails the check +def check_for_match_arg(content: str, filename: str) -> bool: + tree = ast.parse(content) + visitor = MatchArgForWarningsChecker() + visitor.visit(tree) + + if len(visitor.error_set) == 0: + return False + + for error in visitor.error_set: + msg = ERROR_MESSAGE.format( + lineno=error[0], + col_offset=error[1], + path=filename, + ) + sys.stdout.write(msg) + + return True + + +def main(argv: Sequence[str] | None = None) -> None: + parser = argparse.ArgumentParser() + parser.add_argument("paths", nargs="*") + + args = parser.parse_args(argv) + isMatchMissing = False + + for filename in args.paths: + with open(filename, encoding="utf-8") as fd: + content = fd.read() + isMatchMissing = check_for_match_arg(content, filename) | isMatchMissing + + if isMatchMissing: + sys.exit(1) + + +if __name__ == "__main__": + main() From d74e92e350da9921df4b2155f361b1ce6e90a90c Mon Sep 17 00:00:00 2001 From: Chaarvi Bansal Date: Tue, 2 Jul 2024 23:48:46 +0100 Subject: [PATCH 02/11] update comments --- scripts/enforce_match_arg_in_assert_produces_warning.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/scripts/enforce_match_arg_in_assert_produces_warning.py b/scripts/enforce_match_arg_in_assert_produces_warning.py index 0c012bfb67d22..395463c30dfe0 100755 --- a/scripts/enforce_match_arg_in_assert_produces_warning.py +++ b/scripts/enforce_match_arg_in_assert_produces_warning.py @@ -1,6 +1,3 @@ -# 30th June - spent 1hr 20min -# 1st July - spent close to 1hr 30min - """ Enforce that all usages of tms.assert_produces_warning use From 2562c7927211df11b56b60aef0a83e74fbe89dfa Mon Sep 17 00:00:00 2001 From: Chaarvi Bansal Date: Tue, 2 Jul 2024 23:58:02 +0100 Subject: [PATCH 03/11] fix typo --- scripts/enforce_match_arg_in_assert_produces_warning.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/scripts/enforce_match_arg_in_assert_produces_warning.py b/scripts/enforce_match_arg_in_assert_produces_warning.py index 395463c30dfe0..0b4084c9ba953 100755 --- a/scripts/enforce_match_arg_in_assert_produces_warning.py +++ b/scripts/enforce_match_arg_in_assert_produces_warning.py @@ -1,10 +1,10 @@ """ -Enforce that all usages of tms.assert_produces_warning use +Enforce that all usages of tm.assert_produces_warning use the "match" argument. This will help ensure that users always see the correct warning message. -tms.assert_produces_warning(None) is excluded as no warning is +tm.assert_produces_warning(None) is excluded as no warning is produced. """ @@ -32,7 +32,7 @@ def visit_Call(self, node) -> None: # only check for attribute function of class/module tm if ( isinstance(node.func.value, ast.Name) and node.func.value.id == "tm" ): - # ignore tms.assert_produces_warning(None)/tms.assert_produces_warning() + # ignore tm.assert_produces_warning(None)/tm.assert_produces_warning() if ( len(node.args) == 0 or (isinstance(node.args[0], ast.Constant) and node.args[0].value is None) ): From c6cdc2d085d6c1a2e47a37f1c54ef6b5f0bf44d7 Mon Sep 17 00:00:00 2001 From: Chaarvi Bansal Date: Wed, 3 Jul 2024 20:45:05 +0100 Subject: [PATCH 04/11] exclude tm.assert_produces_warning(False) --- scripts/enforce_match_arg_in_assert_produces_warning.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/enforce_match_arg_in_assert_produces_warning.py b/scripts/enforce_match_arg_in_assert_produces_warning.py index 0b4084c9ba953..6d666910b1311 100755 --- a/scripts/enforce_match_arg_in_assert_produces_warning.py +++ b/scripts/enforce_match_arg_in_assert_produces_warning.py @@ -35,7 +35,7 @@ def visit_Call(self, node) -> None: # ignore tm.assert_produces_warning(None)/tm.assert_produces_warning() if ( len(node.args) == 0 or (isinstance(node.args[0], ast.Constant) and - node.args[0].value is None) ): + ( node.args[0].value is None or node.args[0].value is False))): return if not any(keyword.arg == "match" for keyword in node.keywords): self.error_set.append((node.lineno, node.col_offset)) From fde98254c671b93b05e94341745ef6bb9a48e7c8 Mon Sep 17 00:00:00 2001 From: Chaarvi Bansal Date: Wed, 3 Jul 2024 20:52:14 +0100 Subject: [PATCH 05/11] update exclude list --- .pre-commit-config.yaml | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index c150edec50306..b2c463f9f4a22 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -315,7 +315,6 @@ repos: pandas/tests/plotting/test_datetimelike.py| pandas/tests/plotting/frame/test_frame_color.py| pandas/tests/plotting/test_hist_method.py| - pandas/tests/plotting/test_boxplot_method.py| pandas/tests/util/test_deprecate_kwarg.py| pandas/tests/util/test_deprecate.py| pandas/tests/util/test_assert_produces_warning.py| @@ -330,27 +329,20 @@ repos: pandas/tests/indexes/datetimes/methods/test_shift.py| pandas/tests/extension/test_period.py| pandas/tests/io/pytables/test_retain_attributes.py| - pandas/tests/scalar/timestamp/methods/test_round.py| pandas/tests/scalar/timestamp/methods/test_to_pydatetime.py| pandas/tests/io/test_sql.py| - pandas/tests/reshape/test_pivot.py| pandas/tests/plotting/frame/test_hist_box_by.py| + pandas/tests/plotting/test_boxplot_method.py| pandas/tests/frame/methods/test_reindex_like.py| - pandas/tests/plotting/test_series.py| - pandas/tests/io/parser/test_unsupported.py| pandas/tests/series/accessors/test_cat_accessor.py| - pandas/tests/test_algos.py| pandas/tests/indexing/multiindex/test_multiindex.py| pandas/tests/indexes/multi/test_drop.py| - pandas/tests/plotting/frame/test_frame.py| - pandas/tests/window/test_numba.py| - pandas/tests/reshape/test_cut.py| + pandas/tests/io/parser/test_unsupported.py| pandas/tests/apply/test_str.py| pandas/tests/indexes/multi/test_indexing.py| pandas/tests/io/pytables/test_store.py| pandas/tests/io/pytables/test_put.py| pandas/tests/io/pytables/test_round_trip.py| - pandas/tests/io/excel/test_readers.py| pandas/tests/resample/test_datetime_index.py| pandas/tests/io/parser/test_c_parser_only.py| pandas/tests/io/test_stata.py| @@ -361,7 +353,6 @@ repos: pandas/tests/series/methods/test_reindex_like.py| pandas/tests/extension/test_sparse.py| pandas/tests/indexes/test_common.py| - pandas/tests/indexes/datetimes/methods/test_round.py| pandas/tests/indexing/multiindex/test_loc.py| pandas/tests/frame/indexing/test_insert.py| pandas/tests/groupby/test_groupby.py| From 7c4caf24f1c5aac1a358c54eb561f1770eb417bc Mon Sep 17 00:00:00 2001 From: Chaarvi Bansal Date: Wed, 3 Jul 2024 21:24:05 +0100 Subject: [PATCH 06/11] update comments --- scripts/enforce_match_arg_in_assert_produces_warning.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/scripts/enforce_match_arg_in_assert_produces_warning.py b/scripts/enforce_match_arg_in_assert_produces_warning.py index 6d666910b1311..7540b4b95f128 100755 --- a/scripts/enforce_match_arg_in_assert_produces_warning.py +++ b/scripts/enforce_match_arg_in_assert_produces_warning.py @@ -4,7 +4,8 @@ the "match" argument. This will help ensure that users always see the correct warning message. -tm.assert_produces_warning(None) is excluded as no warning is +tm.assert_produces_warning(None), tm.assert_produces_warning() +and tm.assert_produces_warning(False) are excluded as no warning is produced. """ @@ -32,7 +33,8 @@ def visit_Call(self, node) -> None: # only check for attribute function of class/module tm if ( isinstance(node.func.value, ast.Name) and node.func.value.id == "tm" ): - # ignore tm.assert_produces_warning(None)/tm.assert_produces_warning() + # ignore tm.assert_produces_warning(None),tm.assert_produces_warning() + # and tm.assert_produces_warning(False) if ( len(node.args) == 0 or (isinstance(node.args[0], ast.Constant) and ( node.args[0].value is None or node.args[0].value is False))): From cdd630358eab1603f65ce66192d66d8453229fa9 Mon Sep 17 00:00:00 2001 From: Chaarvi Bansal <49508554+chaarvii@users.noreply.github.com> Date: Thu, 4 Jul 2024 09:11:25 +0100 Subject: [PATCH 07/11] Update scripts/enforce_match_arg_in_assert_produces_warning.py Co-authored-by: Abdulaziz Aloqeely <52792999+Aloqeely@users.noreply.github.com> --- scripts/enforce_match_arg_in_assert_produces_warning.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/scripts/enforce_match_arg_in_assert_produces_warning.py b/scripts/enforce_match_arg_in_assert_produces_warning.py index 7540b4b95f128..08bf950cd3733 100755 --- a/scripts/enforce_match_arg_in_assert_produces_warning.py +++ b/scripts/enforce_match_arg_in_assert_produces_warning.py @@ -13,7 +13,10 @@ import argparse import ast -from collections.abc import Sequence +from typing import TYPE_CHECKING +... +if TYPE_CHECKING: + from collections.abc import Sequence import sys ERROR_MESSAGE = ( From 1bfe27ec55ee1ba80cfa3df6cf3aed50f2314759 Mon Sep 17 00:00:00 2001 From: Chaarvi Bansal Date: Thu, 4 Jul 2024 09:13:53 +0100 Subject: [PATCH 08/11] as per reviews --- .../enforce_match_arg_in_assert_produces_warning.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/scripts/enforce_match_arg_in_assert_produces_warning.py b/scripts/enforce_match_arg_in_assert_produces_warning.py index 7540b4b95f128..7915706f20c09 100755 --- a/scripts/enforce_match_arg_in_assert_produces_warning.py +++ b/scripts/enforce_match_arg_in_assert_produces_warning.py @@ -1,5 +1,4 @@ """ - Enforce that all usages of tm.assert_produces_warning use the "match" argument. This will help ensure that users always see the correct warning message. @@ -8,6 +7,9 @@ and tm.assert_produces_warning(False) are excluded as no warning is produced. +This is meant to be run as a pre-commit hook - to run it manually, you can do: + + pre-commit run enforce-match-arg-in-assert-produces-warning --all-files """ @@ -68,14 +70,14 @@ def main(argv: Sequence[str] | None = None) -> None: parser.add_argument("paths", nargs="*") args = parser.parse_args(argv) - isMatchMissing = False + is_match_missing = False for filename in args.paths: with open(filename, encoding="utf-8") as fd: content = fd.read() - isMatchMissing = check_for_match_arg(content, filename) | isMatchMissing + is_match_missing = check_for_match_arg(content, filename) | is_match_missing - if isMatchMissing: + if is_match_missing: sys.exit(1) From b745226811734e53e83feabb58219ddcb24e1760 Mon Sep 17 00:00:00 2001 From: Chaarvi Bansal Date: Thu, 4 Jul 2024 21:23:32 +0100 Subject: [PATCH 09/11] trigger pre-commit From 87dd8c3c4a37ac659f13c5871eae83a554de2494 Mon Sep 17 00:00:00 2001 From: Chaarvi Bansal Date: Thu, 4 Jul 2024 21:25:59 +0100 Subject: [PATCH 10/11] uncommit suggestion --- scripts/enforce_match_arg_in_assert_produces_warning.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/scripts/enforce_match_arg_in_assert_produces_warning.py b/scripts/enforce_match_arg_in_assert_produces_warning.py index 26d0576e478e3..7915706f20c09 100755 --- a/scripts/enforce_match_arg_in_assert_produces_warning.py +++ b/scripts/enforce_match_arg_in_assert_produces_warning.py @@ -15,10 +15,7 @@ import argparse import ast -from typing import TYPE_CHECKING -... -if TYPE_CHECKING: - from collections.abc import Sequence +from collections.abc import Sequence import sys ERROR_MESSAGE = ( From b700b717c8b5be0f150a7c0365cb18754578f937 Mon Sep 17 00:00:00 2001 From: Chaarvi Bansal Date: Fri, 5 Jul 2024 22:52:19 +0100 Subject: [PATCH 11/11] fix sequence import --- scripts/enforce_match_arg_in_assert_produces_warning.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/scripts/enforce_match_arg_in_assert_produces_warning.py b/scripts/enforce_match_arg_in_assert_produces_warning.py index 7915706f20c09..d1a2e9cc2e6dc 100755 --- a/scripts/enforce_match_arg_in_assert_produces_warning.py +++ b/scripts/enforce_match_arg_in_assert_produces_warning.py @@ -11,12 +11,15 @@ pre-commit run enforce-match-arg-in-assert-produces-warning --all-files """ - +from __future__ import annotations import argparse import ast -from collections.abc import Sequence import sys +from typing import TYPE_CHECKING + +if TYPE_CHECKING: + from collections.abc import Sequence ERROR_MESSAGE = ( "{path}:{lineno}:{col_offset}: "