diff --git a/common/djangoapps/student/roles.py b/common/djangoapps/student/roles.py index 7020bcd5d3..daf3f6ee02 100644 --- a/common/djangoapps/student/roles.py +++ b/common/djangoapps/student/roles.py @@ -157,7 +157,6 @@ class CourseRole(GroupBasedRole): # direct copy from auth.authz.get_all_course_role_groupnames will refactor to one impl asap groupnames = [] - # pylint: disable=no-member if isinstance(self.location, Location): try: groupnames.append('{0}_{1}'.format(role, self.location.course_id)) @@ -193,8 +192,6 @@ class OrgRole(GroupBasedRole): A named role in a particular org """ def __init__(self, role, location): - # pylint: disable=no-member - location = Location(location) super(OrgRole, self).__init__(['{}_{}'.format(role, location.org)]) diff --git a/common/lib/xmodule/xmodule/conditional_module.py b/common/lib/xmodule/xmodule/conditional_module.py index 9a2332d556..e296af5aa5 100644 --- a/common/lib/xmodule/xmodule/conditional_module.py +++ b/common/lib/xmodule/xmodule/conditional_module.py @@ -224,7 +224,7 @@ class ConditionalDescriptor(ConditionalFields, SequenceDescriptor): if child.tag == 'show': location = ConditionalDescriptor.parse_sources(child, system) children.extend(location) - show_tag_list.extend(location.url()) + show_tag_list.extend(location.url()) # pylint: disable=no-member else: try: descriptor = system.process_xml(etree.tostring(child)) diff --git a/common/lib/xmodule/xmodule/error_module.py b/common/lib/xmodule/xmodule/error_module.py index 4a513aad95..b8d8d2ba87 100644 --- a/common/lib/xmodule/xmodule/error_module.py +++ b/common/lib/xmodule/xmodule/error_module.py @@ -134,7 +134,7 @@ class ErrorDescriptor(ErrorFields, XModuleDescriptor): ) @classmethod - def from_xml(cls, xml_data, system, id_generator, + def from_xml(cls, xml_data, system, id_generator, # pylint: disable=arguments-differ error_msg='Error not available'): '''Create an instance of this descriptor from the supplied data. diff --git a/common/lib/xmodule/xmodule/modulestore/__init__.py b/common/lib/xmodule/xmodule/modulestore/__init__.py index 0a1cf9ee10..89b8e80d15 100644 --- a/common/lib/xmodule/xmodule/modulestore/__init__.py +++ b/common/lib/xmodule/xmodule/modulestore/__init__.py @@ -268,7 +268,9 @@ class Location(_LocationBase): _check_location_part(value, INVALID_CHARS_NAME) else: _check_location_part(value, INVALID_CHARS) - return super(Location, self)._replace(**kwargs) + + # namedtuple is an old-style class, so don't use super + return _LocationBase._replace(self, **kwargs) def replace(self, **kwargs): ''' diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_location.py b/common/lib/xmodule/xmodule/modulestore/tests/test_location.py index 5f4983e1bf..f0c815f41c 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_location.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_location.py @@ -19,6 +19,9 @@ GENERAL_PAIRS = [ @ddt.ddt class TestLocations(TestCase): + """ + Tests of :class:`.Location` + """ @ddt.data( "tag://org/course/category/name", "tag://org/course/category/name@revision" @@ -173,6 +176,8 @@ class TestLocations(TestCase): loc.course_id # pylint: disable=pointless-statement def test_replacement(self): + # pylint: disable=protected-access + self.assertEquals( Location('t://o/c/c/n@r')._replace(name='new_name'), Location('t://o/c/c/new_name@r'), diff --git a/common/lib/xmodule/xmodule/modulestore/xml.py b/common/lib/xmodule/xmodule/modulestore/xml.py index 13079bf9b3..664cbd9807 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml.py +++ b/common/lib/xmodule/xmodule/modulestore/xml.py @@ -18,10 +18,9 @@ from xmodule.error_module import ErrorDescriptor from xmodule.errortracker import make_error_tracker, exc_info_to_str from xmodule.course_module import CourseDescriptor from xmodule.mako_module import MakoDescriptorSystem -from xmodule.x_module import XMLParsingSystem, prefer_xmodules, policy_key +from xmodule.x_module import XMLParsingSystem, policy_key from xmodule.html_module import HtmlDescriptor -from xblock.core import XBlock from xblock.fields import ScopeIds from xblock.field_data import DictFieldData from xblock.runtime import DictKeyValueStore, IdReader, IdGenerator @@ -222,6 +221,7 @@ class ImportSystem(XMLParsingSystem, MakoDescriptorSystem): # load_item should actually be get_instance, because it expects the course-specific # policy to be loaded. For now, just add the course_id here... def load_item(location): + """Return the XBlock for the specified location""" return xmlstore.get_instance(course_id, Location(location)) resources_fs = OSFS(xmlstore.data_dir / course_dir) @@ -509,6 +509,9 @@ class XMLModuleStore(ModuleStoreReadBase): course_id = CourseDescriptor.make_id(org, course, url_name) def get_policy(usage_id): + """ + Return the policy dictionary to be applied to the specified XBlock usage + """ return policy.get(policy_key(usage_id), {}) system = ImportSystem( @@ -570,7 +573,7 @@ class XMLModuleStore(ModuleStoreReadBase): html = f.read().decode('utf-8') # tabs are referenced in policy.json through a 'slug' which is just the filename without the .html suffix slug = os.path.splitext(os.path.basename(filepath))[0] - loc = course_descriptor.scope_ids.usage_id._replace(category=category, name=slug) + loc = course_descriptor.scope_ids.usage_id.replace(category=category, name=slug) module = system.construct_xblock_from_class( HtmlDescriptor, # We're loading a descriptor, so student_id is meaningless diff --git a/common/lib/xmodule/xmodule/tests/test_import.py b/common/lib/xmodule/xmodule/tests/test_import.py index 994603e2e1..12f889d897 100644 --- a/common/lib/xmodule/xmodule/tests/test_import.py +++ b/common/lib/xmodule/xmodule/tests/test_import.py @@ -78,6 +78,7 @@ class BaseCourseTestCase(unittest.TestCase): class GenericXBlock(XBlock): + """XBlock for testing pure xblock xml import""" has_children = True field1 = String(default="something", scope=Scope.user_state) field2 = Integer(scope=Scope.user_state) @@ -85,6 +86,9 @@ class GenericXBlock(XBlock): @ddt.ddt class PureXBlockImportTest(BaseCourseTestCase): + """ + Tests of import pure XBlocks (not XModules) from xml + """ def assert_xblocks_are_good(self, block): """Assert a number of conditions that must be true for `block` to be good.""" diff --git a/common/lib/xmodule/xmodule/tests/test_xml_module.py b/common/lib/xmodule/xmodule/tests/test_xml_module.py index 62ebf4dce0..d8192d7a8d 100644 --- a/common/lib/xmodule/xmodule/tests/test_xml_module.py +++ b/common/lib/xmodule/xmodule/tests/test_xml_module.py @@ -119,13 +119,11 @@ class InheritingFieldDataTest(unittest.TestCase): parent = self.get_a_block(usage_id="parent") parent.inherited = "Changed!" self.assertEqual(parent.inherited, "Changed!") - parent_id = "parent" for child_num in range(10): usage_id = "child_{}".format(child_num) child = self.get_a_block(usage_id=usage_id) child.parent = "parent" self.assertEqual(child.inherited, "Changed!") - parent_id = usage_id def test_not_inherited(self): # Fields not in the inherited_names list won't be inherited. diff --git a/common/lib/xmodule/xmodule/xml_module.py b/common/lib/xmodule/xmodule/xml_module.py index c659b7787b..d4ea3b17ee 100644 --- a/common/lib/xmodule/xmodule/xml_module.py +++ b/common/lib/xmodule/xmodule/xml_module.py @@ -6,8 +6,7 @@ import sys from lxml import etree from xblock.fields import Dict, Scope, ScopeIds -from xmodule.x_module import XModuleDescriptor, policy_key -from xmodule.modulestore import Location +from xmodule.x_module import XModuleDescriptor from xmodule.modulestore.inheritance import own_metadata, InheritanceKeyValueStore from xmodule.modulestore.xml_exporter import EdxJSONEncoder from xblock.runtime import KvsFieldData @@ -176,7 +175,7 @@ class XmlDescriptor(XModuleDescriptor): return etree.parse(file_object, parser=edx_xml_parser).getroot() @classmethod - def load_file(cls, filepath, fs, def_id): + def load_file(cls, filepath, fs, def_id): # pylint: disable=invalid-name ''' Open the specified file in fs, and call cls.file_to_xml on it, returning the lxml object. @@ -184,8 +183,8 @@ class XmlDescriptor(XModuleDescriptor): Add details and reraise on error. ''' try: - with fs.open(filepath) as file: - return cls.file_to_xml(file) + with fs.open(filepath) as xml_file: + return cls.file_to_xml(xml_file) except Exception as err: # Add info about where we are, but keep the traceback msg = 'Unable to load file contents at path %s for item %s: %s ' % ( diff --git a/pylintrc b/pylintrc index 5a3a1ad01c..cba312005f 100644 --- a/pylintrc +++ b/pylintrc @@ -127,7 +127,13 @@ generated-members= build, # For xblocks fields, - +# For locations + tag, + org, + course, + category, + name, + revision, [BASIC]