Lint the OpenCue Python libraries. (#890)

ci/pylintrc_test

@@ -69,6 +69,7 @@ disable=duplicate-code,
         no-self-use,
         protected-access,
         raise-missing-from,
+        too-many-locals,
         too-many-public-methods,
         unused-argument,
         unused-variable,

ci/run_python_tests.sh

@@ -22,6 +22,10 @@ python rqd/setup.py test
 
 # Some environments don't have pylint available, for ones that do they should pass this flag.
 if [[ "$1" == "--lint" ]]; then
+  cd pycue && python -m pylint --rcfile=../ci/pylintrc_main FileSequence && cd ..
+  cd pycue && python -m pylint --rcfile=../ci/pylintrc_main opencue --ignore=opencue/compiled_proto && cd ..
+  cd pycue && python -m pylint --rcfile=../ci/pylintrc_test tests && cd ..
+
   cd pyoutline && PYTHONPATH=../pycue python -m pylint --rcfile=../ci/pylintrc_main outline && cd ..
   cd pyoutline && PYTHONPATH=../pycue python -m pylint --rcfile=../ci/pylintrc_test tests && cd ..
 

pycue/FileSequence/FrameRange.py

@@ -12,6 +12,12 @@
 #  See the License for the specific language governing permissions and
 #  limitations under the License.
 
+"""
+Helper class for representing a frame range.
+
+It supports a complex syntax implementing features such as comma-separated frame ranges,
+stepped frame ranges and more. See the FrameRange class for more detail.
+"""
 
 from __future__ import division
 from __future__ import print_function
@@ -25,11 +31,12 @@
 
 
 class FrameRange(object):
-    """Represents a sequence of image frames."""
+    """Represents a sequence of frame numbers."""
 
     SINGLE_FRAME_PATTERN = re.compile(r'^(-?)\d+$')
     SIMPLE_FRAME_RANGE_PATTERN = re.compile(r'^(?P<sf>(-?)\d+)-(?P<ef>(-?)\d+)$')
-    STEP_PATTERN = re.compile(r'^(?P<sf>(-?)\d+)-(?P<ef>(-?)\d+)(?P<stepSep>[xy])(?P<step>(-?)\d+)$')
+    STEP_PATTERN = re.compile(
+        r'^(?P<sf>(-?)\d+)-(?P<ef>(-?)\d+)(?P<stepSep>[xy])(?P<step>(-?)\d+)$')
     INTERLEAVE_PATTERN = re.compile(r'^(?P<sf>(-?)\d+)-(?P<ef>(-?)\d+):(?P<step>(-?)\d+)$')
 
     def __init__(self, frameRange):
@@ -103,11 +110,20 @@ def getAll(self):
         return self.frameList
 
     def normalize(self):
+        """Sorts and deduplicates the sequence."""
         self.frameList = list(set(self.frameList))
         self.frameList.sort()
 
     @classmethod
     def parseFrameRange(cls, frameRange):
+        """
+        Parse a string representation into a numerical sequence.
+
+        :type frameRange: str
+        :param frameRange: String representation of the frame range.
+        :rtype: FrameRange
+        :return: FrameRange representing the numerical sequence.
+        """
         singleFrameMatcher = re.match(cls.SINGLE_FRAME_PATTERN, frameRange)
         if singleFrameMatcher:
             return [int(frameRange)]
@@ -116,56 +132,57 @@ def parseFrameRange(cls, frameRange):
         if simpleRangeMatcher:
             startFrame = int(simpleRangeMatcher.group('sf'))
             endFrame = int(simpleRangeMatcher.group('ef'))
-            return cls.getIntRange(startFrame, endFrame, (1 if endFrame >= startFrame else -1))
+            return cls.__getIntRange(startFrame, endFrame, (1 if endFrame >= startFrame else -1))
 
         rangeWithStepMatcher = re.match(cls.STEP_PATTERN, frameRange)
         if rangeWithStepMatcher:
             startFrame = int(rangeWithStepMatcher.group('sf'))
             endFrame = int(rangeWithStepMatcher.group('ef'))
             step = int(rangeWithStepMatcher.group('step'))
             stepSep = rangeWithStepMatcher.group('stepSep')
-            return cls.getSteppedRange(startFrame, endFrame, step, stepSep == 'y')
+            return cls.__getSteppedRange(startFrame, endFrame, step, stepSep == 'y')
 
         rangeWithInterleaveMatcher = re.match(cls.INTERLEAVE_PATTERN, frameRange)
         if rangeWithInterleaveMatcher:
             startFrame = int(rangeWithInterleaveMatcher.group('sf'))
             endFrame = int(rangeWithInterleaveMatcher.group('ef'))
             step = int(rangeWithInterleaveMatcher.group('step'))
-            return cls.getInterleavedRange(startFrame, endFrame, step)
+            return cls.__getInterleavedRange(startFrame, endFrame, step)
 
         raise ValueError('unrecognized frame range syntax ' + frameRange)
 
     @staticmethod
-    def getIntRange(start, end, step):
+    def __getIntRange(start, end, step):
         return list(range(start, end+(step // abs(step)), step))
 
     @classmethod
-    def getSteppedRange(cls, start, end, step, inverseStep):
-        cls.validateStepSign(start, end, step)
-        steppedRange = cls.getIntRange(start, end, step)
+    def __getSteppedRange(cls, start, end, step, inverseStep):
+        cls.__validateStepSign(start, end, step)
+        steppedRange = cls.__getIntRange(start, end, step)
         if inverseStep:
-            fullRange = cls.getIntRange(start, end, (-1 if step < 0 else 1))
+            fullRange = cls.__getIntRange(start, end, (-1 if step < 0 else 1))
             return [frame for frame in fullRange if frame not in steppedRange]
         return steppedRange
 
     @classmethod
-    def getInterleavedRange(cls, start, end, step):
-        cls.validateStepSign(start, end, step)
+    def __getInterleavedRange(cls, start, end, step):
+        cls.__validateStepSign(start, end, step)
         interleavedFrames = OrderedDict()
         incrValue = step // abs(step)
         while abs(step) > 0:
-            interleavedFrames.update([(frame, None) for frame in cls.getIntRange(start, end, step)])
+            interleavedFrames.update(
+                [(frame, None) for frame in cls.__getIntRange(start, end, step)])
             start += incrValue
             step = int(step / 2.0)
         return list(interleavedFrames.keys())
 
     @staticmethod
-    def validateStepSign(start, end, step):
+    def __validateStepSign(start, end, step):
         if step > 1 and end < start:
             raise ValueError(
                 'end frame may not be less than start frame when using a positive step')
-        elif step == 0:
+        if step == 0:
             raise ValueError('step cannot be zero')
-        elif step < 0 and end >= start:
+        if step < 0 and end >= start:
             raise ValueError(
                 'end frame may not be greater than start frame when using a negative step')

pycue/FileSequence/FrameSet.py

@@ -12,6 +12,7 @@
 #  See the License for the specific language governing permissions and
 #  limitations under the License.
 
+"""Module for `FileSequence.FrameSet`."""
 
 from __future__ import absolute_import
 from __future__ import print_function
@@ -23,7 +24,7 @@
 
 
 class FrameSet(object):
-    """Represents a sequence of FrameRanges."""
+    """Represents a sequence of `FileSequence.FrameRange`."""
 
     def __init__(self, frameRange):
         """Construct a FrameSet object by parsing a spec.
@@ -67,11 +68,20 @@ def getAll(self):
         return self.frameList
 
     def normalize(self):
+        """Sorts and dedeuplicates the sequence."""
         self.frameList = list(set(self.frameList))
         self.frameList.sort()
 
     @staticmethod
     def parseFrameRange(frameRange):
+        """
+        Parses a string representation of a frame range into a FrameSet.
+
+        :type frameRange: str
+        :param frameRange: String representation of the frame range.
+        :rtype: FrameSet
+        :return: The FrameSet representing the same sequence.
+        """
         frameList = list()
         for frameRangeSection in frameRange.split(','):
             frameList.extend(FrameRange.parseFrameRange(frameRangeSection))

pycue/FileSequence/__init__.py

@@ -1,3 +1,23 @@
+#  Copyright Contributors to the OpenCue Project
+#
+#  Licensed under the Apache License, Version 2.0 (the "License");
+#  you may not use this file except in compliance with the License.
+#  You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+#  Unless required by applicable law or agreed to in writing, software
+#  distributed under the License is distributed on an "AS IS" BASIS,
+#  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+#  See the License for the specific language governing permissions and
+#  limitations under the License.
+
+"""
+Top-level module of FileSequence.
+
+FileSequence contains helper classes for representing a job's frame range.
+"""
+
 from __future__ import absolute_import
 from __future__ import print_function
 from __future__ import division

pycue/opencue/__init__.py

@@ -12,21 +12,25 @@
 #  See the License for the specific language governing permissions and
 #  limitations under the License.
 
+"""Top level of the opencue module."""
 
 from __future__ import absolute_import
 from __future__ import print_function
 from __future__ import division
 
 import logging
 
+# pylint: disable=cyclic-import
 from .cuebot import Cuebot
 from . import api
 from . import wrappers
 from . import search
 
 from .exception import CueException
 from .exception import EntityNotFoundException
+# pylint: disable=redefined-builtin
 from .util import id
+# pylint: enable=redefined-builtin
 from .util import logPath
 from .util import proxy
 from .util import rep

pycue/opencue/api.py

@@ -12,19 +12,12 @@
 #  See the License for the specific language governing permissions and
 #  limitations under the License.
 
+"""The OpenCue static API."""
 
-
-"""
-The opencue Static API.  This is exported into the package namespace.
-
-Project: opencue Library
-"""
 from __future__ import absolute_import
 from __future__ import print_function
 from __future__ import division
 
-from . import search
-from . import util
 from opencue.compiled_proto import comment_pb2
 from opencue.compiled_proto import criterion_pb2
 from opencue.compiled_proto import cue_pb2
@@ -42,6 +35,7 @@
 from opencue.compiled_proto import subscription_pb2
 from opencue.compiled_proto import task_pb2
 from .cuebot import Cuebot
+# pylint: disable=cyclic-import
 from .wrappers.allocation import Allocation
 from .wrappers.comment import Comment
 from .wrappers.depend import Depend
@@ -60,11 +54,13 @@
 from .wrappers.show import Show
 from .wrappers.subscription import Subscription
 from .wrappers.task import Task
+from . import search
+from . import util
 
 
 __protobufs = [comment_pb2, criterion_pb2, cue_pb2, department_pb2, depend_pb2, facility_pb2,
-               filter_pb2, host_pb2, job_pb2, renderPartition_pb2, report_pb2, service_pb2, show_pb2,
-               subscription_pb2, task_pb2]
+               filter_pb2, host_pb2, job_pb2, renderPartition_pb2, report_pb2, service_pb2,
+               show_pb2, subscription_pb2, task_pb2]
 
 __wrappers = [Action, Allocation, Comment, Depend, Filter, Frame, Group, Host, Job, Layer, Matcher,
               NestedHost, Proc, Show, Subscription, Task]
@@ -319,7 +315,7 @@ def getJobs(**options):
     For example::
 
         # returns only pipe jobs.
-        getJobs(show=["pipe"]) 
+        getJobs(show=["pipe"])
 
     Possible args:
         - job: job names - list
@@ -391,8 +387,6 @@ def getJobNames(**options):
     """Returns a list of job names that match the search parameters.
     See getJobs for the job query options.
 
-    :type  options: dict
-    :param options: a variable list of search criteria
     :rtype:  list
     :return: List of matching str job names"""
     criteria = search.JobSearch.criteriaFromOptions(**options)
@@ -557,15 +551,15 @@ def getHost(uniq):
 # Owners
 #
 @util.grpcExceptionParser
-def getOwner(id):
+def getOwner(owner_id):
     """Return an Owner object from the ID or name.
 
-    :type  id: str
-    :param id: a unique owner identifier or name
+    :type  owner_id: str
+    :param owner_id: a unique owner identifier or name
     :rtype:  Owner
     :return: An Owner object"""
     return Owner(Cuebot.getStub('owner').GetOwner(
-        host_pb2.OwnerGetOwnerRequest(name=id), timeout=Cuebot.Timeout).owner)
+        host_pb2.OwnerGetOwnerRequest(name=owner_id), timeout=Cuebot.Timeout).owner)
 
 #
 # Filters
@@ -640,25 +634,56 @@ def getAllocation(allocId):
 
 @util.grpcExceptionParser
 def deleteAllocation(alloc):
+    """Deletes an allocation.
+
+    :type  alloc: facility_pb2.Allocation
+    :param alloc: allocation to delete
+    :rtype:  facility_pb2.AllocDeleteResponse
+    :return: empty response"""
     return Cuebot.getStub('allocation').Delete(
         facility_pb2.AllocDeleteRequest(allocation=alloc), timeout=Cuebot.Timeout)
 
 
 @util.grpcExceptionParser
 def allocSetBillable(alloc, is_billable):
+    """Sets an allocation billable or not.
+
+    :type  alloc: facility_pb2.Allocation
+    :param alloc: allocation to set
+    :type  is_billable: bool
+    :param is_billable: whether alloc should be billable or not
+    :rtype:  facility_pb2.AllocSetBillableResponse
+    :return: empty response
+    """
     return Cuebot.getStub('allocation').SetBillable(
         facility_pb2.AllocSetBillableRequest(allocation=alloc, value=is_billable),
         timeout=Cuebot.Timeout)
 
 
 @util.grpcExceptionParser
 def allocSetName(alloc, name):
+    """Sets an allocation name.
+
+    :type  alloc: facility_pb2.Allocation
+    :param alloc: allocation to set
+    :type  name: str
+    :param name: new name for the allocation
+    :rtype:  facility_pb2.AllocSetNameResponse
+    :return: empty response"""
     return Cuebot.getStub('allocation').SetName(
         facility_pb2.AllocSetNameRequest(allocation=alloc, name=name), timeout=Cuebot.Timeout)
 
 
 @util.grpcExceptionParser
 def allocSetTag(alloc, tag):
+    """Sets an allocation tag.
+
+    :type  alloc: facility_pb2.Allocation
+    :param alloc: allocation to tag
+    :type  tag: str
+    :param tag: new tag
+    :rtype:  facility_pb2.AllocSetTagResponse
+    :return: empty response"""
     return Cuebot.getStub('allocation').SetTag(
         facility_pb2.AllocSetTagRequest(allocation=alloc, tag=tag), timeout=Cuebot.Timeout)