style: code cleanups from Steven Burch (#29292)

* chore: update deprecated import from collections

* chore: remove outdated imports from markdown library

as it hasn't been supported since 2.0.3 and we're on 3.x.
This was deprecated at least as early as 2012!

* docs: add docstring and remove lint-amnesty to markdown plugin

* chore: remove deprecated etree import

* style: remove unnecessary-comprehension for sets

* style: resolve a number of amnestied pylint complaints

Co-authored-by: stvn <stvn@mit.edu>
This commit is contained in:
Ned Batchelder
2021-11-10 07:11:57 -08:00
committed by GitHub
parent 1d2319b42a
commit d9dd10dc97
30 changed files with 71 additions and 88 deletions

View File

@@ -255,10 +255,7 @@ class CourseQualityView(DeveloperErrorViewMixin, GenericAPIView):
len(cls._get_children(block)) == 0
)
return [
block for block in # lint-amnesty, pylint: disable=unnecessary-comprehension
traverse_pre_order(unit, cls._get_visible_children, leaf_filter)
]
return list(traverse_pre_order(unit, cls._get_visible_children, leaf_filter))
def _stats_dict(self, data): # lint-amnesty, pylint: disable=missing-function-docstring
if not data:

View File

@@ -46,12 +46,7 @@ class Command(BaseCommand):
with open(file_path) as csv_file:
csv_reader = csv.reader(csv_file)
email_mappings = [
(current_email, new_email)
for (current_email, new_email) # lint-amnesty, pylint: disable=unnecessary-comprehension
in csv_reader
]
email_mappings = list(csv_reader)
successful_updates = []
failed_updates = []

View File

@@ -8,7 +8,7 @@ from .symmath_check import symmath_check
class SymmathCheckTest(TestCase): # lint-amnesty, pylint: disable=missing-class-docstring
def test_symmath_check_integers(self):
number_list = [i for i in range(-100, 100)] # lint-amnesty, pylint: disable=unnecessary-comprehension
number_list = range(-100, 100)
self._symmath_check_numbers(number_list)
def test_symmath_check_floats(self):

View File

@@ -194,7 +194,7 @@ class TestModulestoreAssetSize(unittest.TestCase):
results = asset_collection.map_reduce(mapper, reducer, "size_results")
result_str = "{} - Store: {:<15} - Num Assets: {:>6} - Result: {}\n".format(
self.test_run_time, SHORT_NAME_MAP[source_ms], num_assets, [r for r in results.find()] # lint-amnesty, pylint: disable=unnecessary-comprehension
self.test_run_time, SHORT_NAME_MAP[source_ms], num_assets, results.find()
)
with open("bson_sizes.txt", "a") as f:
f.write(result_str)

View File

@@ -432,7 +432,7 @@ class SharedModuleStoreTestCase(
for Django ORM models that will get cleaned up properly.
"""
# Tell Django to clean out all databases, not just default
databases = {alias for alias in connections} # lint-amnesty, pylint: disable=unnecessary-comprehension
databases = set(connections)
@classmethod
@contextmanager
@@ -527,7 +527,7 @@ class ModuleStoreTestCase(
CREATE_USER = True
# Tell Django to clean out all databases, not just default
databases = {alias for alias in connections} # lint-amnesty, pylint: disable=unnecessary-comprehension
databases = set(connections)
@classmethod
def setUpClass(cls):

View File

@@ -553,11 +553,7 @@ class ImportTestCase(BaseCourseTestCase): # lint-amnesty, pylint: disable=missi
# Expect to find an error/exception about characters in "®esources"
expect = "InvalidKeyError"
errors = [
(msg, err)
for msg, err # lint-amnesty, pylint: disable=unnecessary-comprehension
in modulestore.get_course_errors(course.id)
]
errors = modulestore.get_course_errors(course.id)
assert any(((expect in msg) or (expect in err)) for (msg, err) in errors)
chapters = course.get_children()

View File

@@ -58,6 +58,8 @@ class VideoStudentViewHandlers:
"""
Handlers for video module instance.
"""
global_speed = None
transcript_language = None
def handle_ajax(self, dispatch, data):
"""
@@ -97,7 +99,7 @@ class VideoStudentViewHandlers:
setattr(self, key, value)
if key == 'speed':
self.global_speed = self.speed # lint-amnesty, pylint: disable=attribute-defined-outside-init
self.global_speed = self.speed
return json.dumps({'success': True})
@@ -238,14 +240,14 @@ class VideoStudentViewHandlers:
Return value: JSON response (200 on success, 400 for malformed data)
"""
completion_service = self.runtime.service(self, 'completion')
if completion_service is None: # lint-amnesty, pylint: disable=no-else-raise
if completion_service is None:
raise JsonHandlerError(500, "No completion service found")
elif not completion_service.completion_tracking_enabled():
if not completion_service.completion_tracking_enabled():
raise JsonHandlerError(404, "Completion tracking is not enabled and API calls are unexpected")
if not isinstance(data['completion'], (int, float)): # lint-amnesty, pylint: disable=no-else-raise
if not isinstance(data['completion'], (int, float)):
message = "Invalid completion value {}. Must be a float in range [0.0, 1.0]"
raise JsonHandlerError(400, message.format(data['completion']))
elif not 0.0 <= data['completion'] <= 1.0:
if not 0.0 <= data['completion'] <= 1.0:
message = "Invalid completion value {}. Must be in range [0.0, 1.0]"
raise JsonHandlerError(400, message.format(data['completion']))
self.runtime.publish(self, "completion", data)
@@ -321,8 +323,8 @@ class VideoStudentViewHandlers:
log.info("Video: transcript facilities are not available for given language.")
return Response(status=404)
if language != self.transcript_language: # lint-amnesty, pylint: disable=access-member-before-definition
self.transcript_language = language # lint-amnesty, pylint: disable=attribute-defined-outside-init
if language != self.transcript_language:
self.transcript_language = language
try:
if is_bumper:
@@ -451,9 +453,9 @@ class VideoStudioViewHandlers:
elif (
data['language_code'] != data['new_language_code'] and data['new_language_code'] in available_translations
):
error = _('A transcript with the "{language_code}" language code already exists.'.format( # lint-amnesty, pylint: disable=translation-of-non-string
language_code=data['new_language_code']
))
error = _('A transcript with the "{language_code}" language code already exists.').format(
language_code=data['new_language_code'],
)
elif 'file' not in data:
error = _('A transcript file is required.')

View File

@@ -56,7 +56,7 @@ class StudioApiFixture:
Log in as a staff user, then return the cookies for the session (as a dict)
Raises a `StudioApiLoginError` if the login fails.
"""
return {key: val for key, val in self.session.cookies.items()} # lint-amnesty, pylint: disable=unnecessary-comprehension
return dict(self.session.cookies.items())
@lazy
def headers(self):

View File

@@ -61,7 +61,7 @@ class ConfigModelFixture:
Log in as a staff user, then return the cookies for the session (as a dict)
Raises a `ConfigModelFixtureError` if the login fails.
"""
return {key: val for key, val in self.session.cookies.items()} # lint-amnesty, pylint: disable=unnecessary-comprehension
return dict(self.session.cookies.items())
@lazy
def headers(self):

View File

@@ -55,7 +55,7 @@ class FieldOverridePerformanceTestCase(FieldOverrideTestMixin, ProceduralCourseT
"""
__test__ = False
# Tell Django to clean out all databases, not just default
databases = {alias for alias in connections} # lint-amnesty, pylint: disable=unnecessary-comprehension
databases = set(connections)
# TEST_DATA must be overridden by subclasses
TEST_DATA = None

View File

@@ -87,7 +87,7 @@ def get_course_goal_options():
Returns the valid options for goal keys, mapped to their translated
strings, as defined by theCourseGoal model.
"""
return {goal_key: goal_text for goal_key, goal_text in GOAL_KEY_CHOICES} # lint-amnesty, pylint: disable=unnecessary-comprehension
return dict(GOAL_KEY_CHOICES)
def get_course_goal_text(goal_key):

View File

@@ -1,14 +1,12 @@
# Source: https://github.com/mayoff/python-markdown-mathjax # lint-amnesty, pylint: disable=missing-module-docstring
"""
Add MathJax Markdown support
Source: https://github.com/mayoff/python-markdown-mathjax
"""
from xml.etree import ElementTree
import markdown
try:
# Markdown 2.1.0 changed from 2.0.3. We try importing the new version first,
# but import the 2.0.3 version if it fails
from markdown.util import etree, AtomicString
except: # lint-amnesty, pylint: disable=bare-except
from markdown import etree, AtomicString
from markdown.util import AtomicString
class MathJaxPattern(markdown.inlinepatterns.Pattern): # lint-amnesty, pylint: disable=missing-class-docstring
@@ -17,7 +15,7 @@ class MathJaxPattern(markdown.inlinepatterns.Pattern): # lint-amnesty, pylint:
markdown.inlinepatterns.Pattern.__init__(self, r'(?<!\\)(\$\$?)(.+?)\2')
def handleMatch(self, m):
el = etree.Element('span')
el = ElementTree.Element('span')
el.text = AtomicString(m.group(2) + m.group(3) + m.group(2))
return el

View File

@@ -127,17 +127,10 @@ Test Gametrailers
>>> markdown.markdown(s, ['video'])
u'<p><object data="http://www.gametrailers.com/remote_wrap.php?mid=58079" height="392" type="application/x-shockwave-flash" width="480"><param name="movie" value="http://www.gametrailers.com/remote_wrap.php?mid=58079" /><param name="allowFullScreen" value="true" /></object></p>'
"""
from xml.etree import ElementTree
import markdown
try:
# Markdown 2.1.0 changed from 2.0.3. We try importing the new version first,
# but import the 2.0.3 version if it fails
from markdown.util import etree
except ImportError:
from markdown import etree
version = "0.1.6"
@@ -254,7 +247,7 @@ class Yahoo(markdown.inlinepatterns.Pattern): # lint-amnesty, pylint: disable=m
width = self.ext.config['yahoo_width'][0]
height = self.ext.config['yahoo_height'][0]
obj = flash_object(url, width, height)
param = etree.Element('param')
param = ElementTree.Element('param')
param.set('name', 'flashVars')
param.set('value', "id={}&vid={}".format(m.group('yahooid'), m.group('yahoovid')))
obj.append(param)
@@ -271,16 +264,16 @@ class Youtube(markdown.inlinepatterns.Pattern): # lint-amnesty, pylint: disable
def flash_object(url, width, height): # lint-amnesty, pylint: disable=missing-function-docstring
obj = etree.Element('object')
obj = ElementTree.Element('object')
obj.set('type', 'application/x-shockwave-flash')
obj.set('width', width)
obj.set('height', height)
obj.set('data', url)
param = etree.Element('param')
param = ElementTree.Element('param')
param.set('name', 'movie')
param.set('value', url)
obj.append(param)
param = etree.Element('param')
param = ElementTree.Element('param')
param.set('name', 'allowFullScreen')
param.set('value', 'true')
obj.append(param)

View File

@@ -111,7 +111,7 @@ class TestStudentModuleStorage(OtherUserFailureTestMixin, TestCase):
other_key_factory = partial(DjangoKeyValueStore.Key, Scope.user_state, 2, LOCATION('usage_id')) # user_id=2, not 1
existing_field_name = "a_field"
# Tell Django to clean out all databases, not just default
databases = {alias for alias in connections} # lint-amnesty, pylint: disable=unnecessary-comprehension
databases = set(connections)
def setUp(self):
super().setUp()
@@ -241,7 +241,7 @@ class TestStudentModuleStorage(OtherUserFailureTestMixin, TestCase):
class TestMissingStudentModule(TestCase): # lint-amnesty, pylint: disable=missing-class-docstring
# Tell Django to clean out all databases, not just default
databases = {alias for alias in connections} # lint-amnesty, pylint: disable=unnecessary-comprehension
databases = set(connections)
def setUp(self):
super().setUp()

View File

@@ -137,7 +137,7 @@ class TestSubmittingProblems(ModuleStoreTestCase, LoginEnrollmentTestCase, Probl
"""
# Tell Django to clean out all databases, not just default
databases = {alias for alias in connections} # lint-amnesty, pylint: disable=unnecessary-comprehension
databases = set(connections)
# arbitrary constant
COURSE_SLUG = "100"
COURSE_NAME = "test_course"
@@ -339,7 +339,7 @@ class TestCourseGrader(TestSubmittingProblems):
Suite of tests for the course grader.
"""
# Tell Django to clean out all databases, not just default
databases = {alias for alias in connections} # lint-amnesty, pylint: disable=unnecessary-comprehension
databases = set(connections)
def basic_setup(self, late=False, reset=False, showanswer=False):
"""
@@ -752,7 +752,7 @@ class TestCourseGrader(TestSubmittingProblems):
class ProblemWithUploadedFilesTest(TestSubmittingProblems):
"""Tests of problems with uploaded files."""
# Tell Django to clean out all databases, not just default
databases = {alias for alias in connections} # lint-amnesty, pylint: disable=unnecessary-comprehension
databases = set(connections)
def setUp(self):
super().setUp()
@@ -808,7 +808,7 @@ class TestPythonGradedResponse(TestSubmittingProblems):
Check that we can submit a schematic and custom response, and it answers properly.
"""
# Tell Django to clean out all databases, not just default
databases = {alias for alias in connections} # lint-amnesty, pylint: disable=unnecessary-comprehension
databases = set(connections)
SCHEMATIC_SCRIPT = dedent("""
# for a schematic response, submission[i] is the json representation

View File

@@ -22,7 +22,7 @@ class TestDjangoUserStateClient(UserStateClientTestBase, ModuleStoreTestCase):
"""
__test__ = True
# Tell Django to clean out all databases, not just default
databases = {alias for alias in connections} # lint-amnesty, pylint: disable=unnecessary-comprehension
databases = set(connections)
def _user(self, user_idx): # lint-amnesty, pylint: disable=arguments-differ
return self.users[user_idx].username

View File

@@ -24,7 +24,7 @@ from lms.djangoapps.courseware.tests.factories import StudentModuleFactory
class TestStudentModuleHistoryBackends(TestCase):
""" Tests of data in CSMH and CSMHE """
# Tell Django to clean out all databases, not just default
databases = {alias for alias in connections} # lint-amnesty, pylint: disable=unnecessary-comprehension
databases = set(connections)
def setUp(self):
super().setUp()

View File

@@ -335,7 +335,7 @@ def register_and_enroll_students(request, course_id): # pylint: disable=too-man
try:
upload_file = request.FILES.get('students_list')
if upload_file.name.endswith('.csv'):
students = [row for row in csv.reader(upload_file.read().decode('utf-8').splitlines())] # lint-amnesty, pylint: disable=unnecessary-comprehension
students = list(csv.reader(upload_file.read().decode('utf-8').splitlines()))
course = get_course_by_id(course_id)
else:
general_errors.append({
@@ -3322,7 +3322,7 @@ def generate_bulk_certificate_exceptions(request, course_id):
try:
upload_file = request.FILES.get('students_list')
if upload_file.name.endswith('.csv'):
students = [row for row in csv.reader(upload_file.read().decode('utf-8').splitlines())] # lint-amnesty, pylint: disable=unnecessary-comprehension
students = list(csv.reader(upload_file.read().decode('utf-8').splitlines()))
else:
general_errors.append(_('Make sure that the file you upload is in CSV format with no '
'extraneous characters or rows.'))

View File

@@ -111,8 +111,7 @@ def profile_distribution(course_id, feature):
raw_choices = UserProfile.LEVEL_OF_EDUCATION_CHOICES
# short name and display name (full) of the choices.
choices = [(short, full)
for (short, full) in raw_choices] + [('no_data', 'No Data')] # lint-amnesty, pylint: disable=unnecessary-comprehension
choices = list(raw_choices) + [('no_data', 'No Data')]
def get_filter(feature, value):
""" Get the orm filter parameters for a feature. """

View File

@@ -354,7 +354,7 @@ class TestReportMixin:
report_path = report_store.path_to(self.course.id, report_csv_filename)
with report_store.storage.open(report_path) as csv_file:
# Expand the dict reader generator so we don't lose it's content
csv_rows = [row for row in unicodecsv.DictReader(csv_file, encoding='utf-8-sig')] # lint-amnesty, pylint: disable=unnecessary-comprehension
csv_rows = list(unicodecsv.DictReader(csv_file, encoding='utf-8-sig'))
if ignore_other_columns:
csv_rows = [

View File

@@ -13,7 +13,7 @@ from lms.djangoapps.instructor_task.models import PROGRESS
log = logging.getLogger(__name__)
# return status for completed tasks and tasks in progress
STATES_WITH_STATUS = [state for state in READY_STATES] + [PROGRESS] # lint-amnesty, pylint: disable=unnecessary-comprehension
STATES_WITH_STATUS = list(READY_STATES) + [PROGRESS]
def _get_instructor_task_status(task_id):

View File

@@ -111,7 +111,7 @@ class LinkProgramEnrollmentSupportView(View):
for item in ext_key_to_username.items()
if item not in link_errors
]
errors = [message for message in link_errors.values()] # lint-amnesty, pylint: disable=unnecessary-comprehension
errors = list(link_errors.values())
return successes, errors

View File

@@ -177,7 +177,10 @@ FEATURES['CERTIFICATES_HTML_VIEW'] = True
########################## Course Discovery #######################
LANGUAGE_MAP = {'terms': {lang: display for lang, display in ALL_LANGUAGES}, 'name': 'Language'} # lint-amnesty, pylint: disable=unnecessary-comprehension
LANGUAGE_MAP = {
'terms': dict(ALL_LANGUAGES),
'name': 'Language',
}
COURSE_DISCOVERY_MEANINGS = {
'org': {
'name': 'Organization',

View File

@@ -223,7 +223,7 @@ def mock_registered_transformers(transformers):
'openedx.core.djangoapps.content.block_structure.transformer_registry.'
'TransformerRegistry.get_registered_transformers'
) as mock_available_transforms:
mock_available_transforms.return_value = {transformer for transformer in transformers} # lint-amnesty, pylint: disable=unnecessary-comprehension
mock_available_transforms.return_value = set(transformers)
yield

View File

@@ -191,7 +191,7 @@ class ContentLibraryXBlockUserStateTest(ContentLibraryContentTestMixin, TestCase
if the library allows direct learning.
"""
databases = {alias for alias in connections} # lint-amnesty, pylint: disable=unnecessary-comprehension
databases = set(connections)
@XBlock.register_temp_plugin(UserStateTestBlock, UserStateTestBlock.BLOCK_TYPE)
def test_default_values(self):

View File

@@ -307,7 +307,7 @@ class AccountLegacyProfileSerializer(serializers.HyperlinkedModelSerializer, Rea
"""
Enforce all languages are unique.
"""
language_proficiencies = [language for language in value] # lint-amnesty, pylint: disable=unnecessary-comprehension
language_proficiencies = list(value)
unique_language_proficiencies = {language["code"] for language in language_proficiencies}
if len(language_proficiencies) != len(unique_language_proficiencies):
raise serializers.ValidationError("The language_proficiencies field must consist of unique languages.")
@@ -317,7 +317,7 @@ class AccountLegacyProfileSerializer(serializers.HyperlinkedModelSerializer, Rea
"""
Enforce only one entry for a particular social platform.
"""
social_links = [social_link for social_link in value] # lint-amnesty, pylint: disable=unnecessary-comprehension
social_links = list(value)
unique_social_links = {social_link["platform"] for social_link in social_links}
if len(social_links) != len(unique_social_links):
raise serializers.ValidationError("The social_links field must consist of unique social platforms.")

View File

@@ -152,10 +152,10 @@ class RetirementTestCase(TestCase):
return [create_retirement_status(UserFactory(), state=state) for state in RetirementState.objects.all()]
def _get_non_dead_end_states(self):
return [state for state in RetirementState.objects.filter(is_dead_end_state=False)] # lint-amnesty, pylint: disable=unnecessary-comprehension
return RetirementState.objects.filter(is_dead_end_state=False)
def _get_dead_end_states(self):
return [state for state in RetirementState.objects.filter(is_dead_end_state=True)] # lint-amnesty, pylint: disable=unnecessary-comprehension
return RetirementState.objects.filter(is_dead_end_state=True)
def fake_requested_retirement(user):

View File

@@ -401,7 +401,7 @@ class EmailOptInListTest(ModuleStoreTestCase):
try:
with open(output_path) as output_file:
reader = csv.DictReader(output_file, fieldnames=self.OUTPUT_FIELD_NAMES)
rows = [row for row in reader] # lint-amnesty, pylint: disable=unnecessary-comprehension
rows = list(reader)
except OSError:
self.fail(f"Could not find or open output file at '{output_path}'")

View File

@@ -2501,11 +2501,11 @@ class RegistrationValidationViewTests(test_utils.ApiTestCase, OpenEdxEventsTestM
)
@ddt.data(
['name', [name for name in testutils.VALID_NAMES]], # lint-amnesty, pylint: disable=unnecessary-comprehension
['email', [email for email in testutils.VALID_EMAILS]], # lint-amnesty, pylint: disable=unnecessary-comprehension, line-too-long
['password', [password for password in testutils.VALID_PASSWORDS]], # lint-amnesty, pylint: disable=unnecessary-comprehension, line-too-long
['username', [username for username in testutils.VALID_USERNAMES]], # lint-amnesty, pylint: disable=unnecessary-comprehension, line-too-long
['country', [country for country in testutils.VALID_COUNTRIES]] # lint-amnesty, pylint: disable=unnecessary-comprehension, line-too-long
['name', list(testutils.VALID_NAMES)],
['email', list(testutils.VALID_EMAILS)],
['password', list(testutils.VALID_PASSWORDS)],
['username', list(testutils.VALID_USERNAMES)],
['country', list(testutils.VALID_COUNTRIES)],
)
@ddt.unpack
def test_positive_validation_decision(self, form_field_name, user_data):
@@ -2519,11 +2519,11 @@ class RegistrationValidationViewTests(test_utils.ApiTestCase, OpenEdxEventsTestM
@ddt.data(
# Skip None type for invalidity checks.
['name', [name for name in testutils.INVALID_NAMES[1:]]], # lint-amnesty, pylint: disable=unnecessary-comprehension, line-too-long
['email', [email for email in testutils.INVALID_EMAILS[1:]]], # lint-amnesty, pylint: disable=unnecessary-comprehension, line-too-long
['password', [password for password in testutils.INVALID_PASSWORDS[1:]]], # lint-amnesty, pylint: disable=unnecessary-comprehension, line-too-long
['username', [username for username in testutils.INVALID_USERNAMES[1:]]], # lint-amnesty, pylint: disable=unnecessary-comprehension, line-too-long
['country', [country for country in testutils.INVALID_COUNTRIES[1:]]] # lint-amnesty, pylint: disable=unnecessary-comprehension, line-too-long
['name', testutils.INVALID_NAMES[1:]],
['email', testutils.INVALID_EMAILS[1:]],
['password', testutils.INVALID_PASSWORDS[1:]],
['username', testutils.INVALID_USERNAMES[1:]],
['country', testutils.INVALID_COUNTRIES[1:]],
)
@ddt.unpack
def test_negative_validation_decision(self, form_field_name, user_data):

View File

@@ -2,7 +2,7 @@
Utilities related to API views
"""
from collections import Sequence # lint-amnesty, pylint: disable=no-name-in-module, deprecated-class
from collections.abc import Sequence
from functools import wraps
from django.core.exceptions import NON_FIELD_ERRORS, ObjectDoesNotExist, ValidationError