Skip to content

Fix pylint violations #2540

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.

Sign up for GitHub

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
cpennington merged 1 commit into from
Feb 10, 2014
Merged

Fix pylint violations #2540

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
10 changes: 4 additions & 6 deletions common/lib/xmodule/xmodule/tests/test_export.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,8 @@ def strip_filenames(descriptor):
if 'filename' in descriptor.xml_attributes:
del descriptor.xml_attributes['filename']

for d in descriptor.get_children():
strip_filenames(d)
for child in descriptor.get_children():
strip_filenames(child)

descriptor.save()

Expand All @@ -68,7 +68,6 @@ class RoundTripTestCase(unittest.TestCase):
Thus we make sure that export and import work properly.
"""


def setUp(self):
self.maxDiff = None
self.temp_dir = mkdtemp()
Expand Down Expand Up @@ -111,8 +110,8 @@ def test_export_roundtrip(self, course_dir, mock_get):
# export to the same directory--that way things like the custom_tags/ folder
# will still be there.
print("Starting export")
fs = OSFS(root_dir)
initial_course.runtime.export_fs = fs.makeopendir(course_dir)
file_system = OSFS(root_dir)
initial_course.runtime.export_fs = file_system.makeopendir(course_dir)
root = lxml.etree.Element('root')

initial_course.add_xml_to_node(root)
Expand Down Expand Up @@ -152,7 +151,6 @@ def test_export_roundtrip(self, course_dir, mock_get):
))



class TestEdxJsonEncoder(unittest.TestCase):
"""
Tests for xml_exporter.EdxJSONEncoder
Expand Down
101 changes: 77 additions & 24 deletions common/lib/xmodule/xmodule/tests/test_xblock_wrappers.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,20 +27,18 @@
from xmodule.modulestore import Location

from xmodule.x_module import ModuleSystem, XModule, XModuleDescriptor, DescriptorSystem
from xmodule.mako_module import MakoDescriptorSystem
from xmodule.annotatable_module import AnnotatableDescriptor
from xmodule.capa_module import CapaDescriptor
from xmodule.course_module import CourseDescriptor
from xmodule.combined_open_ended_module import CombinedOpenEndedDescriptor
from xmodule.discussion_module import DiscussionDescriptor
from xmodule.error_module import ErrorDescriptor
from xmodule.gst_module import GraphicalSliderToolDescriptor
from xmodule.html_module import HtmlDescriptor
from xmodule.peer_grading_module import PeerGradingDescriptor
from xmodule.poll_module import PollDescriptor
from xmodule.word_cloud_module import WordCloudDescriptor
from xmodule.crowdsource_hinter import CrowdsourceHinterDescriptor
from xmodule.video_module import VideoDescriptor
#from xmodule.video_module import VideoDescriptor
from xmodule.seq_module import SequenceDescriptor
from xmodule.conditional_module import ConditionalDescriptor
from xmodule.randomize_module import RandomizeDescriptor
Expand Down Expand Up @@ -100,54 +98,91 @@ def flatten(class_dict):

@use_strategy(BUILD_STRATEGY)
class ModuleSystemFactory(Factory):
"""
Factory to build a test ModuleSystem. Creation is
performed by :func:`xmodule.tests.get_test_system`, so
arguments for that function are valid factory attributes.
"""
FACTORY_FOR = ModuleSystem

@classmethod
def _build(cls, target_class, *args, **kwargs):
def _build(cls, target_class, *args, **kwargs): # pylint: disable=unused-argument
"""See documentation from :meth:`factory.Factory._build`"""
return get_test_system(*args, **kwargs)


@use_strategy(BUILD_STRATEGY)
class DescriptorSystemFactory(Factory):
"""
Factory to build a test DescriptorSystem. Creation is
performed by :func:`xmodule.tests.get_test_descriptor_system`, so
arguments for that function are valid factory attributes.
"""
FACTORY_FOR = DescriptorSystem

@classmethod
def _build(cls, target_class, *args, **kwargs):
def _build(cls, target_class, *args, **kwargs): # pylint: disable=unused-argument
"""See documentation from :meth:`factory.Factory._build`"""
return get_test_descriptor_system(*args, **kwargs)


class LeafModuleRuntimeFactory(ModuleSystemFactory):
pass


class ContainerModuleRuntimeFactory(ModuleSystemFactory):
"""
Factory to generate a ModuleRuntime that generates children when asked
for them, for testing container XModules.
"""
@post_generation
def depth(self, create, depth, **kwargs):
def depth(self, create, depth, **kwargs): # pylint: disable=unused-argument
"""
When `depth` is specified as a Factory parameter, creates a
tree of children with that many levels.
"""
# pylint: disable=no-member
if depth == 0:
self.get_module.side_effect = lambda x: LeafModuleFactory(descriptor_cls=HtmlDescriptor)
else:
self.get_module.side_effect = lambda x: ContainerModuleFactory(descriptor_cls=VerticalDescriptor, depth=depth-1)
self.get_module.side_effect = lambda x: ContainerModuleFactory(descriptor_cls=VerticalDescriptor, depth=depth - 1)

@post_generation
def position(self, create, position=2, **kwargs):
def position(self, create, position=2, **kwargs): # pylint: disable=unused-argument, method-hidden
"""
Update the position attribute of the generated ModuleRuntime.
"""
self.position = position


class ContainerDescriptorRuntimeFactory(DescriptorSystemFactory):
"""
Factory to generate a DescriptorRuntime that generates children when asked
for them, for testing container XModuleDescriptors.
"""
@post_generation
def depth(self, create, depth, **kwargs):
def depth(self, create, depth, **kwargs): # pylint: disable=unused-argument
"""
When `depth` is specified as a Factory parameter, creates a
tree of children with that many levels.
"""
# pylint: disable=no-member
if depth == 0:
self.load_item.side_effect = lambda x: LeafModuleFactory(descriptor_cls=HtmlDescriptor)
else:
self.load_item.side_effect = lambda x: ContainerModuleFactory(descriptor_cls=VerticalDescriptor, depth=depth-1)
self.load_item.side_effect = lambda x: ContainerModuleFactory(descriptor_cls=VerticalDescriptor, depth=depth - 1)

@post_generation
def position(self, create, position=2, **kwargs):
def position(self, create, position=2, **kwargs): # pylint: disable=unused-argument, method-hidden
"""
Update the position attribute of the generated ModuleRuntime.
"""
self.position = position


@use_strategy(BUILD_STRATEGY)
class LeafDescriptorFactory(Factory):
"""
Factory to generate leaf XModuleDescriptors.
"""
# pylint: disable=missing-docstring

@sarina sarina Feb 10, 2014

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Just checking - will this only disable the warning within this class?

If so, I'm fine with it, but if not, I'd prefer something that only disables the warnings in the places you intend.

@cpennington cpennington Feb 10, 2014

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah, it follows python's scoping (afaik).

@sarina sarina Feb 10, 2014

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

awesome thing to know, thanks


FACTORY_FOR = XModuleDescriptor

runtime = SubFactory(DescriptorSystemFactory)
Expand All @@ -159,7 +194,7 @@ def location(self):

@lazy_attribute
def block_type(self):
return self.descriptor_cls.__name__
return self.descriptor_cls.__name__ # pylint: disable=no-member

@lazy_attribute
def definition_id(self):
Expand All @@ -170,7 +205,7 @@ def usage_id(self):
return self.location

@classmethod
def _build(cls, target_class, *args, **kwargs):
def _build(cls, target_class, *args, **kwargs): # pylint: disable=unused-argument
runtime = kwargs.pop('runtime')
desc_cls = kwargs.pop('descriptor_cls')
block_type = kwargs.pop('block_type')
Expand All @@ -187,24 +222,38 @@ def _build(cls, target_class, *args, **kwargs):


class LeafModuleFactory(LeafDescriptorFactory):

"""
Factory to generate leaf XModuleDescriptors that are prepped to be
used as XModules.
"""
@post_generation
def xmodule_runtime(self, create, xmodule_runtime, **kwargs):
def xmodule_runtime(self, create, xmodule_runtime, **kwargs): # pylint: disable=method-hidden, unused-argument
"""
Set the xmodule_runtime to make this XModuleDescriptor usable
as an XModule.
"""
if xmodule_runtime is None:
xmodule_runtime = LeafModuleRuntimeFactory()
xmodule_runtime = ModuleSystemFactory()

self.xmodule_runtime = xmodule_runtime


class ContainerDescriptorFactory(LeafDescriptorFactory):
"""
Factory to generate XModuleDescriptors that are containers.
"""
runtime = SubFactory(ContainerDescriptorRuntimeFactory)
children = range(3)


class ContainerModuleFactory(LeafModuleFactory):
"""
Factory to generate XModuleDescriptors that are containers
and are ready to act as XModules.
"""
@lazy_attribute
def xmodule_runtime(self):
return ContainerModuleRuntimeFactory(depth=self.depth)
return ContainerModuleRuntimeFactory(depth=self.depth) # pylint: disable=no-member


@ddt.ddt
Expand All @@ -223,7 +272,11 @@ def skip_if_invalid(self, descriptor_cls):
"""
pass

def check_property(self, descriptor):
def check_property(self, descriptor): # pylint: disable=unused-argument
"""
Execute assertions to verify that the property under test is true for
the supplied descriptor.
"""
raise SkipTest("check_property not defined")

# Test that for all of the leaf XModule Descriptors,
Expand All @@ -247,13 +300,13 @@ def test_container_node_xmodules_only(self, cls_and_fields):
# Test that when an xmodule is generated from descriptor_cls
# with mixed xmodule and xblock children, the test property holds
@ddt.data(*flatten(CONTAINER_XMODULES))
def test_container_node_mixed(self, cls_and_fields):
def test_container_node_mixed(self, cls_and_fields): # pylint: disable=unused-argument
raise SkipTest("XBlock support in XDescriptor not yet fully implemented")

# Test that when an xmodule is generated from descriptor_cls
# with only xblock children, the test property holds
@ddt.data(*flatten(CONTAINER_XMODULES))
def test_container_node_xblocks_only(self, cls_and_fields):
def test_container_node_xblocks_only(self, cls_and_fields): # pylint: disable=unused-argument
raise SkipTest("XBlock support in XModules not yet fully implemented")


Expand Down
2 changes: 1 addition & 1 deletion pylintrc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ class-rgx=[A-Z_][a-zA-Z0-9]+$
function-rgx=[a-z_][a-z0-9_]{2,30}$

# Regular expression which should only match correct method names
method-rgx=([a-z_][a-z0-9_]{2,60}|setUp|set[Uu]pClass|tearDown|tear[Dd]ownClass|assert[A-Z]\w*)$
method-rgx=([a-z_][a-z0-9_]{2,60}|setUp|set[Uu]pClass|tearDown|tear[Dd]ownClass|assert[A-Z]\w*|maxDiff)$

# Regular expression which should only match correct instance attribute names
attr-rgx=[a-z_][a-z0-9_]{2,30}$
Expand Down