-
-
Notifications
You must be signed in to change notification settings - Fork 18.5k
ENH: move an exception and add a prehook to check for exception place… #48088
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
Merged
mroeschke
merged 13 commits into
pandas-dev:main
from
dataxerik:ENH-move-invalid-version-and-add-prehook
Sep 20, 2022
Merged
Changes from all commits
Commits
Show all changes
13 commits
Select commit
Hold shift + click to select a range
4b1224a
ENH: move an exception and add a prehook to check for exception place…
dataxerik 4c67ef2
ENH: fix import
dataxerik 7f9ea07
ENH: revert moving error
dataxerik b7793ce
ENH: add docstring and fix import for test
dataxerik d40aee7
ENH: re-design approach based on feedback
dataxerik 6d8a141
Merge remote-tracking branch 'origin/main' into ENH-move-invalid-vers…
dataxerik 4e89d88
ENH: update whatsnew rst
dataxerik 323ffd7
ENH: apply feedback changes
dataxerik 0d28679
Merge remote-tracking branch 'origin/main' into ENH-move-invalid-vers…
dataxerik b9ae614
ENH: refactor to remove exception_warning_list and ignore _version.py
dataxerik 75e8eac
Merge remote-tracking branch 'origin/main' into ENH-move-invalid-vers…
dataxerik b5d9948
Merge remote-tracking branch 'origin/main' into ENH-move-invalid-vers…
dataxerik 82dbaa0
ENH: remove NotThisMethod from tests and all
dataxerik File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,59 @@ | ||
import pytest | ||
|
||
from scripts.validate_exception_location import ( | ||
ERROR_MESSAGE, | ||
validate_exception_and_warning_placement, | ||
) | ||
|
||
PATH = "t.py" | ||
|
||
# ERRORS_IN_TESTING_RST is the set returned when parsing testing.rst for all the | ||
# exceptions and warnings. | ||
CUSTOM_EXCEPTION_NOT_IN_TESTING_RST = "MyException" | ||
CUSTOM_EXCEPTION__IN_TESTING_RST = "MyOldException" | ||
ERRORS_IN_TESTING_RST = {CUSTOM_EXCEPTION__IN_TESTING_RST} | ||
|
||
TEST_CODE = """ | ||
import numpy as np | ||
import sys | ||
|
||
def my_func(): | ||
pass | ||
|
||
class {custom_name}({error_type}): | ||
pass | ||
|
||
""" | ||
|
||
|
||
# Test with various python-defined exceptions to ensure they are all flagged. | ||
@pytest.fixture(params=["Exception", "ValueError", "Warning", "UserWarning"]) | ||
def error_type(request): | ||
return request.param | ||
|
||
|
||
def test_class_that_inherits_an_exception_and_is_not_in_the_testing_rst_is_flagged( | ||
capsys, error_type | ||
): | ||
content = TEST_CODE.format( | ||
custom_name=CUSTOM_EXCEPTION_NOT_IN_TESTING_RST, error_type=error_type | ||
) | ||
expected_msg = ERROR_MESSAGE.format(errors=CUSTOM_EXCEPTION_NOT_IN_TESTING_RST) | ||
with pytest.raises(SystemExit, match=None): | ||
validate_exception_and_warning_placement(PATH, content, ERRORS_IN_TESTING_RST) | ||
result_msg, _ = capsys.readouterr() | ||
assert result_msg == expected_msg | ||
|
||
|
||
def test_class_that_inherits_an_exception_but_is_in_the_testing_rst_is_not_flagged( | ||
capsys, error_type | ||
): | ||
content = TEST_CODE.format( | ||
custom_name=CUSTOM_EXCEPTION__IN_TESTING_RST, error_type=error_type | ||
) | ||
validate_exception_and_warning_placement(PATH, content, ERRORS_IN_TESTING_RST) | ||
|
||
|
||
def test_class_that_does_not_inherit_an_exception_is_not_flagged(capsys): | ||
content = "class MyClass(NonExceptionClass): pass" | ||
validate_exception_and_warning_placement(PATH, content, ERRORS_IN_TESTING_RST) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,105 @@ | ||
""" | ||
Validate that the exceptions and warnings are in appropriate places. | ||
|
||
Checks for classes that inherit a python exception and warning and | ||
flags them, unless they are exempted from checking. Exempt meaning | ||
the exception/warning is defined in testing.rst. Testing.rst contains | ||
a list of pandas defined exceptions and warnings. This list is kept | ||
current by other pre-commit hook, pandas_errors_documented.py. | ||
This hook maintains that errors.__init__.py and testing.rst are in-sync. | ||
Therefore, the exception or warning should be defined or imported in | ||
errors.__init__.py. Ideally, the exception or warning is defined unless | ||
there's special reason to import it. | ||
|
||
Prints the exception/warning that do not follow this convention. | ||
|
||
Usage:: | ||
|
||
As a pre-commit hook: | ||
pre-commit run validate-errors-locations --all-files | ||
""" | ||
from __future__ import annotations | ||
|
||
import argparse | ||
import ast | ||
import pathlib | ||
import sys | ||
from typing import Sequence | ||
|
||
API_PATH = pathlib.Path("doc/source/reference/testing.rst").resolve() | ||
ERROR_MESSAGE = ( | ||
"The following exception(s) and/or warning(s): {errors} exist(s) outside of " | ||
"pandas/errors/__init__.py. Please either define them in " | ||
"pandas/errors/__init__.py. Or, if not possible then import them in " | ||
"pandas/errors/__init__.py.\n" | ||
) | ||
|
||
|
||
def get_warnings_and_exceptions_from_api_path() -> set[str]: | ||
with open(API_PATH) as f: | ||
doc_errors = { | ||
line.split(".")[1].strip() for line in f.readlines() if "errors" in line | ||
} | ||
return doc_errors | ||
|
||
|
||
class Visitor(ast.NodeVisitor): | ||
def __init__(self, path: str, exception_set: set[str]) -> None: | ||
self.path = path | ||
self.exception_set = exception_set | ||
self.found_exceptions = set() | ||
|
||
def visit_ClassDef(self, node) -> None: | ||
def is_an_exception_subclass(base_id: str) -> bool: | ||
return ( | ||
base_id == "Exception" | ||
or base_id.endswith("Warning") | ||
or base_id.endswith("Error") | ||
) | ||
|
||
exception_classes = [] | ||
|
||
# Go through the class's bases and check if they are an Exception or Warning. | ||
for base in node.bases: | ||
base_id = getattr(base, "id", None) | ||
if base_id and is_an_exception_subclass(base_id): | ||
exception_classes.append(base_id) | ||
|
||
# The class subclassed an Exception or Warning so add it to the list. | ||
if exception_classes: | ||
self.found_exceptions.add(node.name) | ||
|
||
|
||
def validate_exception_and_warning_placement( | ||
file_path: str, file_content: str, errors: set[str] | ||
): | ||
tree = ast.parse(file_content) | ||
visitor = Visitor(file_path, errors) | ||
visitor.visit(tree) | ||
|
||
misplaced_exceptions = visitor.found_exceptions.difference(errors) | ||
|
||
# If misplaced_exceptions isn't an empty list then there exists | ||
# pandas-defined Exception or Warnings outside of pandas/errors/__init__.py, so | ||
# we should flag them. | ||
if misplaced_exceptions: | ||
msg = ERROR_MESSAGE.format(errors=", ".join(misplaced_exceptions)) | ||
sys.stdout.write(msg) | ||
sys.exit(1) | ||
|
||
|
||
def main(argv: Sequence[str] | None = None) -> None: | ||
parser = argparse.ArgumentParser() | ||
parser.add_argument("paths", nargs="*") | ||
args = parser.parse_args(argv) | ||
|
||
error_set = get_warnings_and_exceptions_from_api_path() | ||
|
||
for path in args.paths: | ||
with open(path, encoding="utf-8") as fd: | ||
content = fd.read() | ||
validate_exception_and_warning_placement(path, content, error_set) | ||
|
||
|
||
if __name__ == "__main__": | ||
main() |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.