Skip to content
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

New configuration option - Allowing "pylint:" commments #973

Merged
merged 20 commits into from
Nov 18, 2023
Merged
Show file tree
Hide file tree
Changes from 17 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).

- Added new configuration option `use-pyta-error-messages` to let users choose whether PythonTA should overwrite pylint's error messages.
- Both PlainReporter and ColorReporter emphasize specific code chunks by using overline characters under any part that is highlighted as ERROR.
- Added new configuration option `allow-pylint-comments` to let users choose whether PythonTA should allow comments beginning with pylint: or not.

## [2.6.4] - 2024-11-10

Expand Down
11 changes: 11 additions & 0 deletions docs/usage/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,17 @@ import python_ta
python_ta.check_all(..., load_default_config=False)
```

## Allowing 'pylint:' Comments

PythonTA allows you to choose whether you want to locally disable checks by using 'pylint:' in a comment, i.e. it
lets you choose whether or not you want to allow comments that begin with 'pylint:'. The default value for this option is False, i.e. PythonTA by default would not allow you to use 'pylint:' in a comment.

```python
import python_ta

python_ta.check_all(..., config={"allow-pylint-comments": True})
```

## Custom Error Messages

By default, PythonTA overwrites some of pylint's error messages with its own to make them more beginner-friendly.
Expand Down
20 changes: 17 additions & 3 deletions python_ta/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,8 @@ def _check(
errs = [] # Errors caught in files for data submission
config = {} # Configuration settings for data submission
for file_py in get_file_paths(locations):
if not _verify_pre_check(file_py):
allowed_pylint = linter.config.allow_pylint_comments
if not _verify_pre_check(file_py, allowed_pylint):
continue # Check the other files
# Load config file in user location. Construct new linter each
# time, so config options don't bleed to unintended files.
Expand Down Expand Up @@ -289,6 +290,15 @@ def reset_linter(
"help": "Path to patch config toml file.",
},
),
(
"allow-pylint-comments",
{
"default": False,
"type": "yn",
"metavar": "<yn>",
"help": "allows or disallows pylint: comments",
},
),
(
"use-pyta-error-messages",
{
Expand Down Expand Up @@ -358,13 +368,17 @@ def get_file_paths(rel_path: AnyStr) -> Generator[AnyStr, None, None]:
yield os.path.join(root, filename) # Format path, from root.


def _verify_pre_check(filepath: AnyStr) -> bool:
"""Check student code for certain issues."""
def _verify_pre_check(filepath: AnyStr, allow_pylint_comments: bool) -> bool:
"""Check student code for certain issues.
The additional allow_pylint_comments parameter indicates whether we want the user to be able to add comments
beginning with pylint which can be used to locally disable checks."""
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Put the ending """ on a new line (this is the style we use when there's a multi-line docstring)

# Make sure the program doesn't crash for students.
# Could use some improvement for better logging and error reporting.
try:
# Check for inline "pylint:" comment, which may indicate a student
# trying to disable a check.
if allow_pylint_comments:
return True
with tokenize.open(os.path.expanduser(filepath)) as f:
for tok_type, content, _, _, _ in tokenize.generate_tokens(f.readline):
if tok_type != tokenize.COMMENT:
Expand Down
14 changes: 14 additions & 0 deletions python_ta/allow_pylint_comments_false_testcase.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
def some_function():
david-yz-liu marked this conversation as resolved.
Show resolved Hide resolved
x = 5 # pylint: disable something
y = 0
result = x / y
return result


a = 10
b = 20
if __name__ == "__main__":
import python_ta

options = {"allow-pylint-comments": False}
python_ta.check_all(config=options)
14 changes: 14 additions & 0 deletions python_ta/allow_pylint_comments_true_testcase.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
def some_function():
x = 5 # pylint: disable something
y = 0
result = x / y
return result


a = 10
b = 20
if __name__ == "__main__":
import python_ta

options = {"allow-pylint-comments": True}
python_ta.check_all(config=options)
3 changes: 3 additions & 0 deletions python_ta/config/.pylintrc
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@ pyta-server-address = http://127.0.0.1:5000
# Path to messages_config toml file
# messages-config-path = messages.toml

# Allowing pylint: comments
allow-pylint-comments = no

# Set whether the default error messages by pylint should be overwritten by PythonTA's custom messages
use-pyta-error-messages = yes

Expand Down
21 changes: 21 additions & 0 deletions tests/test_config/test_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
"""
import json
import os
from unittest.mock import mock_open, patch

import pytest

Expand Down Expand Up @@ -226,3 +227,23 @@ def test_config_parse_error_has_no_snippet() -> None:
snippet = reporter.messages[config][0].snippet

assert snippet == ""


def test_allow_pylint_comments() -> None:
"""Test that checks whether the allow-pylint-comments configuration option works as expected when it is
set to True"""
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As above, make sure the ending """ is on a new line. Same with the second test case below.


with patch("python_ta.tokenize.open", mock_open(read_data="# pylint: disable")):
result = python_ta._verify_pre_check("", allow_pylint_comments=True)

assert result is True


def test_disallows_pylint_comments() -> None:
"""Test that checks whether the allow-pylint-comments configuration option works as expected when it is
is set to False"""

with patch("python_ta.tokenize.open", mock_open(read_data="# pylint: disable")):
result = python_ta._verify_pre_check("", allow_pylint_comments=False)

assert result is False
Loading