From 8671ea3bfd82e9b507aa9d9cb4b51d626a34522e Mon Sep 17 00:00:00 2001 From: Charlie-XIAO Date: Wed, 28 Jun 2023 13:26:59 +0800 Subject: [PATCH 01/10] CI add liniting to check NoDefault used only for typing --- .pre-commit-config.yaml | 5 +++ scripts/validate_unwanted_patterns.py | 47 +++++++++++++++++++++++++++ 2 files changed, 52 insertions(+) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 220465a1d2847..42961566373a2 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -339,6 +339,11 @@ repos: language: python entry: python scripts/validate_unwanted_patterns.py --validation-type="strings_with_wrong_placed_whitespace" types_or: [python, cython] + - id: unwanted-patterns-nodefault-not-used-for-typing + name: Check for pandas._libs.lib.NoDefault not used for typing + language: python + entry: python scripts/validate_unwanted_patterns.py --validation-type="nodefault_not_used_for_typing" + types_or: [python] - id: use-pd_array-in-core name: Import pandas.array as pd_array in core language: python diff --git a/scripts/validate_unwanted_patterns.py b/scripts/validate_unwanted_patterns.py index e171d1825ac48..c03b0fda45f96 100755 --- a/scripts/validate_unwanted_patterns.py +++ b/scripts/validate_unwanted_patterns.py @@ -353,6 +353,52 @@ def has_wrong_whitespace(first_line: str, second_line: str) -> bool: ) +def nodefault_not_used_for_typing(file_obj: IO[str]) -> Iterable[Tuple[int, str]]: + """Test case where pandas._libs.lib.NoDefault is not used for typing. + + Parameters + ---------- + file_obj : IO + File-like object containing the Python code to validate. + + Yields + ------ + line_number : int + Line number of unconcatenated string. + msg : str + Explanation of the error. + """ + contents = file_obj.read() + tree = ast.parse(contents) + for node in ast.walk(tree): + for child in ast.iter_child_nodes(node): + child.parent = node + + def child_of(node, types): + """Check if any ancestor of the node has the specified type. + + Parameters + ---------- + node : AST + A node in the AST. + types : tuple of classes + The types as in `ininstance(xxx, types)`. + """ + curnode = getattr(node, "parent", None) + while curnode is not None: + if isinstance(curnode, types): + return True + curnode = getattr(curnode, "parent", None) + return False + + for node in ast.walk(tree): + if (isinstance(node, ast.Name) and node.id == "NoDefault") or ( + isinstance(node, ast.Attribute) and node.attr == "NoDefault" + ): + if not child_of(node, (ast.AnnAssign, ast.arg)): + yield (node.lineno, "NoDefault is not used for typing") + + def main( function: Callable[[IO[str]], Iterable[Tuple[int, str]]], source_path: str, @@ -405,6 +451,7 @@ def main( "private_function_across_module", "private_import_across_module", "strings_with_wrong_placed_whitespace", + "nodefault_not_used_for_typing", ] parser = argparse.ArgumentParser(description="Unwanted patterns checker.") From fd13ae3b4107227e1e6f39a7c6e8b7e1b2cfb0d0 Mon Sep 17 00:00:00 2001 From: Charlie-XIAO Date: Wed, 28 Jun 2023 13:27:54 +0800 Subject: [PATCH 02/10] minor modification --- .pre-commit-config.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 42961566373a2..93e8a1155f66e 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -340,7 +340,7 @@ repos: entry: python scripts/validate_unwanted_patterns.py --validation-type="strings_with_wrong_placed_whitespace" types_or: [python, cython] - id: unwanted-patterns-nodefault-not-used-for-typing - name: Check for pandas._libs.lib.NoDefault not used for typing + name: Check for `pandas._libs.lib.NoDefault` not used for typing language: python entry: python scripts/validate_unwanted_patterns.py --validation-type="nodefault_not_used_for_typing" types_or: [python] From 525e851e58058008feccfc726e43c299a6897cb1 Mon Sep 17 00:00:00 2001 From: Charlie-XIAO Date: Wed, 28 Jun 2023 20:40:50 +0800 Subject: [PATCH 03/10] reduce cost --- scripts/validate_unwanted_patterns.py | 56 +++++++++++++-------------- 1 file changed, 28 insertions(+), 28 deletions(-) diff --git a/scripts/validate_unwanted_patterns.py b/scripts/validate_unwanted_patterns.py index c03b0fda45f96..273468ff0e6bc 100755 --- a/scripts/validate_unwanted_patterns.py +++ b/scripts/validate_unwanted_patterns.py @@ -364,39 +364,39 @@ def nodefault_not_used_for_typing(file_obj: IO[str]) -> Iterable[Tuple[int, str] Yields ------ line_number : int - Line number of unconcatenated string. + Line number of misused lib.NoDefault. msg : str Explanation of the error. """ contents = file_obj.read() tree = ast.parse(contents) - for node in ast.walk(tree): - for child in ast.iter_child_nodes(node): - child.parent = node - - def child_of(node, types): - """Check if any ancestor of the node has the specified type. - - Parameters - ---------- - node : AST - A node in the AST. - types : tuple of classes - The types as in `ininstance(xxx, types)`. - """ - curnode = getattr(node, "parent", None) - while curnode is not None: - if isinstance(curnode, types): - return True - curnode = getattr(curnode, "parent", None) - return False - - for node in ast.walk(tree): - if (isinstance(node, ast.Name) and node.id == "NoDefault") or ( - isinstance(node, ast.Attribute) and node.attr == "NoDefault" + in_annotation = False + nodes: List[tuple[bool, ast.AST]] = [(in_annotation, tree)] + + while nodes: + in_annotation, node = nodes.pop() + if not in_annotation and ( + isinstance(node, ast.Name) # Case `NoDefault` + and node.id == "NoDefault" + or isinstance(node, ast.Attribute) # Cases e.g. `lib.NoDefault` + and node.attr == "NoDefault" ): - if not child_of(node, (ast.AnnAssign, ast.arg)): - yield (node.lineno, "NoDefault is not used for typing") + yield (node.lineno, "NoDefault is not used for typing") + + # This part is adapted from + # https://github.com/asottile/pyupgrade/blob/5495a248f2165941c5d3b82ac3226ba7ad1fa59d/pyupgrade/_data.py#L70-L113 + for name in reversed(node._fields): + value = getattr(node, name) + if name in {"annotation", "returns"}: + next_in_annotation = True + else: + next_in_annotation = in_annotation + if isinstance(value, ast.AST): + nodes.append((next_in_annotation, value)) + elif isinstance(value, list): + for value in reversed(value): + if isinstance(value, ast.AST): + nodes.append((next_in_annotation, value)) def main( @@ -460,7 +460,7 @@ def main( parser.add_argument( "--format", "-f", - default="{source_path}:{line_number}:{msg}", + default="{source_path}:{line_number}: {msg}", help="Output format of the error message.", ) parser.add_argument( From c00b290e19fcebd125cda8ebf79bab528b720ed7 Mon Sep 17 00:00:00 2001 From: Charlie-XIAO Date: Wed, 28 Jun 2023 22:17:30 +0800 Subject: [PATCH 04/10] tests added for the new linting check --- .../tests/test_validate_unwanted_patterns.py | 69 +++++++++++++++++++ 1 file changed, 69 insertions(+) diff --git a/scripts/tests/test_validate_unwanted_patterns.py b/scripts/tests/test_validate_unwanted_patterns.py index 90eca13b21628..0840f5dda257c 100644 --- a/scripts/tests/test_validate_unwanted_patterns.py +++ b/scripts/tests/test_validate_unwanted_patterns.py @@ -375,3 +375,72 @@ def test_strings_with_wrong_placed_whitespace_raises(self, data, expected): validate_unwanted_patterns.strings_with_wrong_placed_whitespace(fd) ) assert result == expected + + +class TestNoDefaultNotUsedForTyping: + @pytest.mark.parametrize( + "data", + [ + ( + """ +def f( + a: int | NoDefault, + b: float | lib.NoDefault = 0.1, + c: pandas._libs.lib.NoDefault = lib.no_default, +) -> lib.NoDefault | None: + pass +""" + ), + ( + """ +# var = lib.NoDefault +# the above is incorrect +a: NoDefault | int +b: lib.NoDefault = lib.no_default +""" + ), + ], + ) + def test_nodefault_not_used_for_typing(self, data): + fd = io.StringIO(data.strip()) + result = list(validate_unwanted_patterns.nodefault_not_used_for_typing(fd)) + assert result == [] + + @pytest.mark.parametrize( + "data, expected", + [ + ( + ( + """ +def f( + a = lib.NoDefault, + b: Any + = pandas._libs.lib.NoDefault, +): + pass +""" + ), + [ + (2, "NoDefault is not used for typing"), + (4, "NoDefault is not used for typing"), + ], + ), + ( + ( + """ +a: Any = lib.NoDefault +if a is NoDefault: + pass +""" + ), + [ + (1, "NoDefault is not used for typing"), + (2, "NoDefault is not used for typing"), + ], + ), + ], + ) + def test_nodefault_not_used_for_typing_raises(self, data, expected): + fd = io.StringIO(data.strip()) + result = list(validate_unwanted_patterns.nodefault_not_used_for_typing(fd)) + assert result == expected From 835e6401879747445793aef9eff9b55748251555 Mon Sep 17 00:00:00 2001 From: Charlie-XIAO Date: Wed, 28 Jun 2023 23:31:43 +0800 Subject: [PATCH 05/10] types_or -> types because only python --- .pre-commit-config.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 93e8a1155f66e..57e5485894350 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -343,7 +343,7 @@ repos: name: Check for `pandas._libs.lib.NoDefault` not used for typing language: python entry: python scripts/validate_unwanted_patterns.py --validation-type="nodefault_not_used_for_typing" - types_or: [python] + types: [python] - id: use-pd_array-in-core name: Import pandas.array as pd_array in core language: python From fce3ffc7af240ea5e32c6962ef91d3ee1ecc5488 Mon Sep 17 00:00:00 2001 From: Charlie-XIAO Date: Wed, 28 Jun 2023 23:45:58 +0800 Subject: [PATCH 06/10] rephrase pre-commit hook name --- .pre-commit-config.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 57e5485894350..9742f57ff4e9e 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -340,7 +340,7 @@ repos: entry: python scripts/validate_unwanted_patterns.py --validation-type="strings_with_wrong_placed_whitespace" types_or: [python, cython] - id: unwanted-patterns-nodefault-not-used-for-typing - name: Check for `pandas._libs.lib.NoDefault` not used for typing + name: Check that `pandas._libs.lib.NoDefault` is only used for typing language: python entry: python scripts/validate_unwanted_patterns.py --validation-type="nodefault_not_used_for_typing" types: [python] From bc8cb7e7b49192e86840ac97ead2974e05e8cbd0 Mon Sep 17 00:00:00 2001 From: Charlie-XIAO Date: Thu, 29 Jun 2023 09:02:24 +0800 Subject: [PATCH 07/10] rephrase more --- .pre-commit-config.yaml | 6 +++--- scripts/tests/test_validate_unwanted_patterns.py | 10 +++++----- scripts/validate_unwanted_patterns.py | 6 +++--- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 9742f57ff4e9e..4379220c33687 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -339,10 +339,10 @@ repos: language: python entry: python scripts/validate_unwanted_patterns.py --validation-type="strings_with_wrong_placed_whitespace" types_or: [python, cython] - - id: unwanted-patterns-nodefault-not-used-for-typing - name: Check that `pandas._libs.lib.NoDefault` is only used for typing + - id: unwanted-patterns-nodefault-used-not-only-for-typing + name: Check that `pandas._libs.lib.NoDefault` is used only for typing language: python - entry: python scripts/validate_unwanted_patterns.py --validation-type="nodefault_not_used_for_typing" + entry: python scripts/validate_unwanted_patterns.py --validation-type="nodefault_used_not_only_for_typing" types: [python] - id: use-pd_array-in-core name: Import pandas.array as pd_array in core diff --git a/scripts/tests/test_validate_unwanted_patterns.py b/scripts/tests/test_validate_unwanted_patterns.py index 0840f5dda257c..95af881b54856 100644 --- a/scripts/tests/test_validate_unwanted_patterns.py +++ b/scripts/tests/test_validate_unwanted_patterns.py @@ -377,7 +377,7 @@ def test_strings_with_wrong_placed_whitespace_raises(self, data, expected): assert result == expected -class TestNoDefaultNotUsedForTyping: +class TestNoDefaultUsedNotOnlyForTyping: @pytest.mark.parametrize( "data", [ @@ -401,9 +401,9 @@ def f( ), ], ) - def test_nodefault_not_used_for_typing(self, data): + def test_nodefault_used_not_only_for_typing(self, data): fd = io.StringIO(data.strip()) - result = list(validate_unwanted_patterns.nodefault_not_used_for_typing(fd)) + result = list(validate_unwanted_patterns.nodefault_used_not_only_for_typing(fd)) assert result == [] @pytest.mark.parametrize( @@ -440,7 +440,7 @@ def f( ), ], ) - def test_nodefault_not_used_for_typing_raises(self, data, expected): + def test_nodefault_used_not_only_for_typing_raises(self, data, expected): fd = io.StringIO(data.strip()) - result = list(validate_unwanted_patterns.nodefault_not_used_for_typing(fd)) + result = list(validate_unwanted_patterns.nodefault_used_not_only_for_typing(fd)) assert result == expected diff --git a/scripts/validate_unwanted_patterns.py b/scripts/validate_unwanted_patterns.py index 273468ff0e6bc..cffae7d18bee1 100755 --- a/scripts/validate_unwanted_patterns.py +++ b/scripts/validate_unwanted_patterns.py @@ -353,7 +353,7 @@ def has_wrong_whitespace(first_line: str, second_line: str) -> bool: ) -def nodefault_not_used_for_typing(file_obj: IO[str]) -> Iterable[Tuple[int, str]]: +def nodefault_used_not_only_for_typing(file_obj: IO[str]) -> Iterable[Tuple[int, str]]: """Test case where pandas._libs.lib.NoDefault is not used for typing. Parameters @@ -381,7 +381,7 @@ def nodefault_not_used_for_typing(file_obj: IO[str]) -> Iterable[Tuple[int, str] or isinstance(node, ast.Attribute) # Cases e.g. `lib.NoDefault` and node.attr == "NoDefault" ): - yield (node.lineno, "NoDefault is not used for typing") + yield (node.lineno, "NoDefault is used not only for typing") # This part is adapted from # https://github.com/asottile/pyupgrade/blob/5495a248f2165941c5d3b82ac3226ba7ad1fa59d/pyupgrade/_data.py#L70-L113 @@ -451,7 +451,7 @@ def main( "private_function_across_module", "private_import_across_module", "strings_with_wrong_placed_whitespace", - "nodefault_not_used_for_typing", + "nodefault_used_not_only_for_typing", ] parser = argparse.ArgumentParser(description="Unwanted patterns checker.") From 5b848edbf9327fedcd4fb4284e3ce5944a92f96a Mon Sep 17 00:00:00 2001 From: Charlie-XIAO Date: Thu, 29 Jun 2023 11:11:58 +0800 Subject: [PATCH 08/10] fix failing tests: --- scripts/tests/test_validate_unwanted_patterns.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/scripts/tests/test_validate_unwanted_patterns.py b/scripts/tests/test_validate_unwanted_patterns.py index 95af881b54856..b4423197e2573 100644 --- a/scripts/tests/test_validate_unwanted_patterns.py +++ b/scripts/tests/test_validate_unwanted_patterns.py @@ -421,8 +421,8 @@ def f( """ ), [ - (2, "NoDefault is not used for typing"), - (4, "NoDefault is not used for typing"), + (2, "NoDefault is used not only for typing"), + (4, "NoDefault is used not only for typing"), ], ), ( @@ -434,8 +434,8 @@ def f( """ ), [ - (1, "NoDefault is not used for typing"), - (2, "NoDefault is not used for typing"), + (1, "NoDefault is used not only for typing"), + (2, "NoDefault is used not only for typing"), ], ), ], From 440de031e8915ff2824ee8661c7176c737de5189 Mon Sep 17 00:00:00 2001 From: Charlie-XIAO Date: Thu, 29 Jun 2023 12:33:07 +0800 Subject: [PATCH 09/10] retrigger checks From 29d1a421d5f0f7ad1d4fa27a14d43a2228f5e551 Mon Sep 17 00:00:00 2001 From: Charlie-XIAO Date: Thu, 29 Jun 2023 17:34:34 +0800 Subject: [PATCH 10/10] retrigger checks