Merge pull request #1015 from benfitzpatrick/996.feedback-other

#119: implement some feedback from #996.

bin/cylc-cycletime

@@ -134,22 +134,22 @@ else:
     n_chosen = 0
 
     if options.print_year:
-        n_chosen +=1
+        n_chosen += 1
         if options.num_expanded_year_digits:
             template = u"±XCCYY"
         else:
             template = "CCYY"
 
     if options.print_month:
-        n_chosen +=1
+        n_chosen += 1
         template = "MM"
 
     if options.print_day:
-        n_chosen +=1
+        n_chosen += 1
         template = "DD"
 
     if options.print_hour:
-        n_chosen +=1
+        n_chosen += 1
         template = "%H"
 
     if n_chosen != 0 and n_chosen != 1:
@@ -177,7 +177,7 @@ except ValueError:
     try:
         cycle_point = cylc.cycling.iso8601.point_parse(cycle_point_string)
     except ValueError as exc:
-        parser.error( 'ERROR: invalid cycle: ' + str(exc) )
+        parser.error( 'ERROR: invalid cycle: %s' % exc )
 
 offset_props = {}
 
@@ -217,7 +217,7 @@ if options.offset:
         offset += isodatetime.parsers.TimeIntervalParser().parse(
             opt_offset) * sign_factor
     except ValueError as exc:
-        parser.error( 'ERROR: offset not valid: ' + str(exc) )
+        parser.error( 'ERROR: offset not valid: %s' % exc )
 cycle_point += offset
 if template is None:
     print cycle_point

bin/cylc-monitor

@@ -146,6 +146,7 @@ try:
 
             task_ids = states.keys()
             n_tasks_all = len( task_ids )
+            task_ids.sort()
 
             seen_time = {}
             seen_name = {}

bin/cylc-restart

@@ -143,14 +143,14 @@ where they got to while the suite was down."""
 
         nlines = len(lines)
         if nlines == 0:
-            raise Exception( "ERROR, empty suite state dump: " + file_name )
+            raise Exception( "ERROR, empty suite state dump: %s" % file_name )
         elif nlines < 3:
-            print >> sys.stderr, ("ERROR, The suite state dump contains only",
-                                  nlines, "lines:")
+            print >> sys.stderr, (
+                "ERROR, The suite state dump contains only %d lines" % nlines)
             for l in lines:
                 print ' ', l.rstrip()
             raise Exception(
-                "ERROR, incomplete suite state dump: " + file_name)
+                "ERROR, incomplete suite state dump: %s" % file_name)
 
         index = 0
         # run mode : <mode>
@@ -169,8 +169,8 @@ where they got to while the suite was down."""
 
         if self.run_mode == 'live' and old_run_mode != 'live':
             raise Exception(
-                "ERROR: cannot RESTART in " + self.run_mode + " from a " +
-                old_run_mode + " state dump"
+                "ERROR: cannot RESTART in %s from a %s state dump" % (
+                    self.run_mode, old_run_mode)
             )
 
         state_start_string = None
@@ -218,7 +218,7 @@ where they got to while the suite was down."""
         if line != 'Begin task states':
             raise Exception(
                 "ERROR, illegal state dump line " +
-                "(expected 'Begin task states'): " + line
+                "(expected 'Begin task states'): %s" % line
             )
 
         index += 1
@@ -262,9 +262,17 @@ where they got to while the suite was down."""
                 if self.start_point != state_start_point:
                     # the state dump doesn't lie about start cycles
                     if self.options.ignore_startcycle:
-                        print >> sys.stderr, "WARNING: ignoring old initial cycle", state_start_point, "; using suite.rc", self.start_point
+                        print >> sys.stderr, (
+                            "WARNING: ignoring old initial cycle " +
+                            "%s; using suite.rc %s" % (
+                                state_start_point, self.start_point)
+                        )
                     else:
-                        print >> sys.stderr, "WARNING: old initial cycle", state_start_point, "overriding suite.rc", self.start_point
+                        print >> sys.stderr, (
+                            "WARNING: old initial cycle " +
+                            "%s, overriding suite.rc %s" % (
+                                state_start_point, self.start_point)
+                        )
                         self.start_point = state_start_point
             else:
                 # reinstate the former start cycle
@@ -282,7 +290,10 @@ where they got to while the suite was down."""
             elif self.stop_point is not None:
                 # a stop cycle was given on the restart command line or suite.rc file
                 if self.stop_point != state_stop_point:
-                    print >> sys.stderr, "WARNING: overriding the old stop cycle", state_stop_point, "with", self.stop_point
+                    print >> sys.stderr, (
+                        "WARNING: overriding the old stop cycle "
+                        "%s with %s" % (state_stop_point, self.stop_point)
+                    )
             else:
                 # reinstate the old stop cycle
                 self.stop_point = state_stop_point
@@ -380,12 +391,17 @@ where they got to while the suite was down."""
                 )
             except TaskNotDefinedError, x:
                 print >> sys.stderr, str(x)
-                print >> sys.stderr, "WARNING: ignoring task", name, "from the suite state dump file"
+                print >> sys.stderr, (
+                    "WARNING: ignoring task %s " % name +
+                    "from the suite state dump file")
                 print >> sys.stderr, "(the task definition has probably been deleted from the suite)."
                 continue
             except Exception, x:
                 print >> sys.stderr, str(x)
-                print >> sys.stderr, "ERROR: could not load task", name, "from the suite state dump file"
+                print >> sys.stderr, (
+                    "ERROR: could not load task %s " % name +
+                    "from the suite state dump file"
+                )
                 # TODO: Is it safe to have "raise x" here?
                 continue
 
@@ -415,18 +431,26 @@ where they got to while the suite was down."""
                     user_at_host = row[0]
                     self.old_user_at_host_set.add(str(user_at_host))
                 else:
-                    print >> sys.stderr, "WARNING:", id, "failed to read user@host from run-db!"
+                    print >> sys.stderr, (
+                        "WARNING: %s " % id +
+                        "failed to read user@host from run-db!"
+                    )
 
                 # get submit_method_id and try_num from run-db
                 submit_method_id = try_num = None
                 row = self.db.get_task_submit_method_id_and_try(name, tag)
                 if row and row[0]:
                     submit_method_id, try_num = row
                 else:
-                    print >> sys.stderr, "WARNING:", id, "failed to read submit_method_id and try_num from run-db!"
+                    print >> sys.stderr, (
+                        "WARNING: %s " % id +
+                        "failed to read submit_method_id and try_num from " +
+                        "run-db!"
+                    )
 
                 if None in [ user_at_host, submit_method_id, try_num ]:
-                    print >> sys.stderr, "WARNING: cannot determine what happened to " + id
+                    print >> sys.stderr, (
+                        "WARNING: cannot determine what happened to %s" % id)
                 else:
                     # update the task proxy with submit ID etc.
                     itask.submit_method_id = submit_method_id
@@ -453,7 +477,8 @@ where they got to while the suite was down."""
                 itask.outputs.set_all_completed()
 
             else:
-                raise Exception( 'ERROR: unknown task state for ' + itask.id )
+                raise Exception(
+                    'ERROR: unknown task state for %s' % itask.id)
 
             self.pool.add_to_runahead_pool( itask )
 

bin/cylc-stop

@@ -167,40 +167,18 @@ except Exception, x:
 method = None
 if options.wall_clock:
     method = 'stop after clock time'
-    prompt_text = 'Set shutdown at wall clock ' + options.wall_clock
+    prompt_text = 'Set shutdown at wall clock %s' % options.wall_clock
     shutdown_arg = options.wall_clock
 elif shutdown_at:
     # STOP argument detected
-    if not cylc.TaskID.is_valid_id( shutdown_arg ):
-        # not a task ID
-        # not a task ID or a date time;
-        # TODO ISO - MOVE THIS LOGIC INTO SCHEDULER AND TEST
-        try:
-            # is it a cycle point?
-            ##ct(shutdown_arg)
-            pass
-        except CycleTimeError,x:
-            # nope: is it an async integer tag?
-            try:
-                int( shutdown_arg )
-            except ValueError:
-                # nope: not task ID, date time, or CYCLE_POINT
-                raise SystemExit( "ERROR:, invalid STOP argument: " + shutdown_arg )
-            else:
-                # is probably an async CYCLE_POINT
-                method = 'stop after tag'
-                prompt_text = 'Set shutdown at cycle point ' + shutdown_arg
-                #print 'async'
-        else:
-            # is a cycle point
-            method = 'stop after tag'
-            prompt_text = 'Set shutdown at cycle point ' + shutdown_arg
-            #print 'cycling'
-    else:
+    if cylc.TaskID.is_valid_id( shutdown_arg ):
         # is a task ID
         method = 'stop after task'
-        prompt_text = 'Set shutdown after task ' + shutdown_arg
-        #print 'task id'
+        prompt_text = 'Set shutdown after task %s' % shutdown_arg
+    else:
+        # not a task ID, may be a cycle point
+        method = 'stop after tag'
+        prompt_text = 'Set shutdown at cycle point %s' % shutdown_arg
 
 if method:
     prompt( prompt_text + ' for ' + suite, options.force )
@@ -212,7 +190,7 @@ if method:
         sys.exit(x)
 
 elif options.now:
-    prompt( 'Shut down ' + suite + ' now', options.force )
+    prompt( 'Shut down %s now' % suite, options.force )
     try:
         result = proxy.put( 'stop now' )
     except Exception,x:
@@ -221,7 +199,7 @@ elif options.now:
         sys.exit(x)
 
 elif options.quick:
-    prompt( 'Shut down ' + suite + ' quickly', options.force )
+    prompt( 'Shut down %s quickly' % suite, options.force )
     try:
         result = proxy.put( 'stop quickly' )
     except Exception,x:
@@ -230,7 +208,7 @@ elif options.quick:
         sys.exit(x)
 
 else:
-    prompt( 'Shut down ' + suite, options.force )
+    prompt( 'Shut down %s' % suite, options.force )
     print "Telling the suite to shut down ..."
     try:
         result = proxy.put( 'stop cleanly', options.kill )

lib/cylc/config.py

@@ -871,16 +871,15 @@ def check_tasks( self ):
                 # instantiate a task
                 itask = self.taskdefs[name].get_task_class()( tag, 'waiting', None, True, validate=True )
             except TypeError, x:
-                raise
                 # This should not happen as we now explicitly catch use
                 # of synchronous special tasks in an asynchronous graph.
                 # But in principle a clash of multiply inherited base
                 # classes due to choice of "special task" modifiers
                 # could cause a TypeError.
-                raise SuiteConfigError, '(inconsistent use of special tasks?)'
+                raise SuiteConfigError('(inconsistent use of special tasks?)')
             except Exception, x:
-                raise
-                raise SuiteConfigError, 'ERROR, failed to instantiate task ' + str(name)
+                raise SuiteConfigError(
+                    'ERROR, failed to instantiate task %s: %s' % (name, x))
             if not itask.tag:
                 if flags.verbose:
                     print " + Task out of bounds for " + str(tag) + ": " + itask.name

lib/cylc/cycling/__init__.py

@@ -26,6 +26,16 @@ def __str__(self):
         return self.ERROR_MESSAGE.format(*self.args)
 
 
+class PointParsingError(ValueError):
+
+    """An error raised when a point has an incorrect value."""
+
+    ERROR_MESSAGE = "Incompatible value for {0}: {1}"
+
+    def __str__(self):
+        return self.ERROR_MESSAGE.format(*self.args)
+
+
 class PointBase(object):
 
     """The base class for single points in a cycler sequence.
@@ -51,7 +61,7 @@ def __init__(self, value):
         self.value = value
 
     def standardise(self):
-        """Format self.value into a standard representation."""
+        """Format self.value into a standard representation and check it."""
         return self
 
     def __str__(self):

lib/cylc/cycling/integer.py

@@ -18,7 +18,7 @@
 
 import re
 
-from cylc.cycling import PointBase, IntervalBase
+from cylc.cycling import PointBase, IntervalBase, PointParsingError
 
 """
 Integer cycling by point, interval, and sequence classes.
@@ -85,6 +85,14 @@ def sub(self, other):
     def add(self, other):
         return IntegerPoint(int(self) + int(other))
 
+    def standardise(self):
+        """Format self.value into a standard representation and check it."""
+        try:
+            self.value = str(int(self))
+        except (TypeError, ValueError):
+            raise PointParsingError(type(self), self.value)
+        return self
+
     def __int__(self):
         return int(self.value)
 
@@ -391,10 +399,6 @@ def init_from_cfg(cfg):
 if __name__ == '__main__':
 
     r = IntegerSequence( 'R/1/P3', 1, 10 )
-    #r = IntegerSequence( 'R/c2/P2', 1, 10 )
-    #r = IntegerSequence( 'R2/c2/P2', 1, 10 )
-    #r = IntegerSequence( 'R2/c4/c6', 1, 10 )
-    #r = IntegerSequence( 'R2/P2/c6', 1, 10 )
 
     r.set_offset( IntegerInterval('4') )
 

lib/cylc/cycling/iso8601.py

@@ -23,7 +23,7 @@
 from isodatetime.timezone import (
     get_local_time_zone, get_local_time_zone_format)
 from cylc.time_parser import CylcTimeParser
-from cylc.cycling import PointBase, IntervalBase
+from cylc.cycling import PointBase, IntervalBase, PointParsingError
 from parsec.validate import IllegalValueError
 
 # TODO - Consider copy vs reference of points, intervals, sequences
@@ -103,7 +103,10 @@ def __cmp__(self, other):
         return self._iso_point_cmp(self.value, other.value)
 
     def standardise(self):
-        self.value = str(point_parse(self.value))
+        try:
+            self.value = str(point_parse(self.value))
+        except ValueError:
+            raise PointParsingError(type(self), self.value)
         return self
 
     def sub(self, other):

lib/cylc/cylc_xdot.py

@@ -424,8 +424,10 @@ def get_graph( self, group_nodes=[], ungroup_nodes=[],
             one = self.ctime
             two = self.stop_after
         else:
-            one = str( self.suiterc.cfg['visualization']['initial cycle point'])
-            two = str(self.suiterc.cfg['visualization']['final cycle point'])
+            one = str(
+                self.suiterc.cfg['visualization']['initial cycle point'])
+            two = str(
+                self.suiterc.cfg['visualization']['final cycle point'])
 
         graph = self.suiterc.get_graph( one, two,
                 raw=self.raw, group_nodes=group_nodes,