Minor cleanups in docstrings and error messages.

Project: http://git-wip-us.apache.org/repos/asf/incubator-beam/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-beam/commit/c1f42e62
Tree: http://git-wip-us.apache.org/repos/asf/incubator-beam/tree/c1f42e62
Diff: http://git-wip-us.apache.org/repos/asf/incubator-beam/diff/c1f42e62

Branch: refs/heads/python-sdk
Commit: c1f42e6203de8d0576dbbf324c22edf6aac0ca64
Parents: 0fa9c4b
Author: Robert Bradshaw <rober...@google.com>
Authored: Tue Sep 13 16:25:33 2016 -0700
Committer: Robert Bradshaw <rober...@google.com>
Committed: Fri Sep 23 13:44:57 2016 -0700

----------------------------------------------------------------------
 sdks/python/apache_beam/io/filebasedsource.py   | 25 +++++++++-----------
 .../apache_beam/io/filebasedsource_test.py      | 10 ++++----
 sdks/python/apache_beam/io/iobase.py            |  4 ++--
 sdks/python/apache_beam/io/range_trackers.py    |  2 +-
 sdks/python/apache_beam/io/source_test_utils.py | 25 ++++++++++----------
 5 files changed, 31 insertions(+), 35 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-beam/blob/c1f42e62/sdks/python/apache_beam/io/filebasedsource.py
----------------------------------------------------------------------
diff --git a/sdks/python/apache_beam/io/filebasedsource.py 
b/sdks/python/apache_beam/io/filebasedsource.py
index 14de140..5204f04 100644
--- a/sdks/python/apache_beam/io/filebasedsource.py
+++ b/sdks/python/apache_beam/io/filebasedsource.py
@@ -54,8 +54,8 @@ class _ConcatSource(iobase.BoundedSource):
     if start_position or stop_position:
       raise ValueError(
           'Multi-level initial splitting is not supported. Expected start and '
-          'stop positions to be None. Received %r and %r respectively.',
-          start_position, stop_position)
+          'stop positions to be None. Received %r and %r respectively.' %
+          (start_position, stop_position))
 
     for source in self._sources:
       # We assume all sub-sources to produce bundles that specify weight using
@@ -224,25 +224,22 @@ class FileBasedSource(iobase.BoundedSource):
 
 
 class _SingleFileSource(iobase.BoundedSource):
-  """Denotes a source for a specific file type.
-
-  This should be sub-classed to add support for reading a new file type.
-  """
+  """Denotes a source for a specific file type."""
 
   def __init__(self, file_based_source, file_name, start_offset, stop_offset,
                min_bundle_size=0):
-    if not (isinstance(start_offset, int) or isinstance(start_offset, long)):
-      raise ValueError(
-          'start_offset must be a number. Received: %r', start_offset)
+    if not isinstance(start_offset, (int, long)):
+      raise TypeError(
+          'start_offset must be a number. Received: %r' % start_offset)
     if stop_offset != range_trackers.OffsetRangeTracker.OFFSET_INFINITY:
-      if not (isinstance(stop_offset, int) or isinstance(stop_offset, long)):
-        raise ValueError(
-            'stop_offset must be a number. Received: %r', stop_offset)
+      if not isinstance(stop_offset, (int, long)):
+        raise TypeError(
+            'stop_offset must be a number. Received: %r' % stop_offset)
       if start_offset >= stop_offset:
         raise ValueError(
             'start_offset must be smaller than stop_offset. Received %d and %d 
'
-            'for start and stop offsets respectively',
-            start_offset, stop_offset)
+            'for start and stop offsets respectively' %
+            (start_offset, stop_offset))
 
     self._file_name = file_name
     self._is_gcs_file = file_name.startswith('gs://') if file_name else False

http://git-wip-us.apache.org/repos/asf/incubator-beam/blob/c1f42e62/sdks/python/apache_beam/io/filebasedsource_test.py
----------------------------------------------------------------------
diff --git a/sdks/python/apache_beam/io/filebasedsource_test.py 
b/sdks/python/apache_beam/io/filebasedsource_test.py
index c4ad026..50f0a22 100644
--- a/sdks/python/apache_beam/io/filebasedsource_test.py
+++ b/sdks/python/apache_beam/io/filebasedsource_test.py
@@ -384,19 +384,19 @@ class TestSingleFileSource(unittest.TestCase):
 
     fbs = LineSource('dymmy_pattern')
 
-    with self.assertRaisesRegexp(ValueError, start_not_a_number_error):
+    with self.assertRaisesRegexp(TypeError, start_not_a_number_error):
       SingleFileSource(
           fbs, file_name='dummy_file', start_offset='aaa', stop_offset='bbb')
-    with self.assertRaisesRegexp(ValueError, start_not_a_number_error):
+    with self.assertRaisesRegexp(TypeError, start_not_a_number_error):
       SingleFileSource(
           fbs, file_name='dummy_file', start_offset='aaa', stop_offset=100)
-    with self.assertRaisesRegexp(ValueError, stop_not_a_number_error):
+    with self.assertRaisesRegexp(TypeError, stop_not_a_number_error):
       SingleFileSource(
           fbs, file_name='dummy_file', start_offset=100, stop_offset='bbb')
-    with self.assertRaisesRegexp(ValueError, stop_not_a_number_error):
+    with self.assertRaisesRegexp(TypeError, stop_not_a_number_error):
       SingleFileSource(
           fbs, file_name='dummy_file', start_offset=100, stop_offset=None)
-    with self.assertRaisesRegexp(ValueError, start_not_a_number_error):
+    with self.assertRaisesRegexp(TypeError, start_not_a_number_error):
       SingleFileSource(
           fbs, file_name='dummy_file', start_offset=None, stop_offset=100)
 

http://git-wip-us.apache.org/repos/asf/incubator-beam/blob/c1f42e62/sdks/python/apache_beam/io/iobase.py
----------------------------------------------------------------------
diff --git a/sdks/python/apache_beam/io/iobase.py 
b/sdks/python/apache_beam/io/iobase.py
index e1f364b..4305fb6 100644
--- a/sdks/python/apache_beam/io/iobase.py
+++ b/sdks/python/apache_beam/io/iobase.py
@@ -545,11 +545,11 @@ class RangeTracker(object):
 
   def start_position(self):
     """Returns the starting position of the current range, inclusive."""
-    raise NotImplementedError
+    raise NotImplementedError(type(self))
 
   def stop_position(self):
     """Returns the ending position of the current range, exclusive."""
-    raise NotImplementedError
+    raise NotImplementedError(type(self))
 
   def try_claim(self, position):  # pylint: disable=unused-argument
     """Atomically determines if a record at a split point is within the range.

http://git-wip-us.apache.org/repos/asf/incubator-beam/blob/c1f42e62/sdks/python/apache_beam/io/range_trackers.py
----------------------------------------------------------------------
diff --git a/sdks/python/apache_beam/io/range_trackers.py 
b/sdks/python/apache_beam/io/range_trackers.py
index af6f6c8..080e2f3 100644
--- a/sdks/python/apache_beam/io/range_trackers.py
+++ b/sdks/python/apache_beam/io/range_trackers.py
@@ -138,10 +138,10 @@ class OffsetRangeTracker(iobase.RangeTracker):
         return
 
       logging.debug('Agreeing to split %r at %d', self, split_offset)
-      self._stop_offset = split_offset
 
       split_fraction = (float(split_offset - self._start_offset) / (
           self._stop_offset - self._start_offset))
+      self._stop_offset = split_offset
 
       return self._stop_offset, split_fraction
 

http://git-wip-us.apache.org/repos/asf/incubator-beam/blob/c1f42e62/sdks/python/apache_beam/io/source_test_utils.py
----------------------------------------------------------------------
diff --git a/sdks/python/apache_beam/io/source_test_utils.py 
b/sdks/python/apache_beam/io/source_test_utils.py
index 4e3a3e3..f52f16b 100644
--- a/sdks/python/apache_beam/io/source_test_utils.py
+++ b/sdks/python/apache_beam/io/source_test_utils.py
@@ -201,43 +201,42 @@ def _assertSplitAtFractionBehavior(
   if split_result is not None:
     if len(split_result) != 2:
       raise ValueError('Split result must be a tuple that contains split '
-                       'position and split fraction. Received: %r',
-                       split_result)
+                       'position and split fraction. Received: %r' %
+                       (split_result,))
 
     if range_tracker.stop_position() != split_result[0]:
       raise ValueError('After a successful split, the stop position of the '
                        'RangeTracker must be the same as the returned split '
                        'position. Observed %r and %r which are different.',
-                       range_tracker.stop_position(), split_result[0])
+                       range_tracker.stop_position() % (split_result[0],))
 
     if split_fraction < 0 or split_fraction > 1:
       raise ValueError('Split fraction must be within the range [0,1]',
-                       'Observed split fraction was %r.', split_result[1])
+                       'Observed split fraction was %r.' % (split_result[1],))
 
   stop_position_after_split = range_tracker.stop_position()
   if split_result and stop_position_after_split == stop_position_before_split:
     raise ValueError('Stop position %r did not change after a successful '
-                     'split of source %r at fraction %r.',
-                     stop_position_before_split, source, split_fraction)
+                     'split of source %r at fraction %r.' %
+                     (stop_position_before_split, source, split_fraction))
 
   if expected_outcome == ExpectedSplitOutcome.MUST_SUCCEED_AND_BE_CONSISTENT:
     if not split_result:
       raise ValueError('Expected split of source %r at fraction %r to be '
                        'successful after reading %d elements. But '
-                       'the split failed.',
-                       source, split_fraction,
-                       num_items_to_read_before_split)
+                       'the split failed.' %
+                       (source, split_fraction, 
num_items_to_read_before_split))
   elif expected_outcome == ExpectedSplitOutcome.MUST_FAIL:
     if split_result:
       raise ValueError('Expected split of source %r at fraction %r after '
                        'reading %d elements to fail. But splitting '
-                       'succeeded with result %r.',
-                       source, split_fraction,
-                       num_items_to_read_before_split, split_result)
+                       'succeeded with result %r.' %
+                       (source, split_fraction, num_items_to_read_before_split,
+                        split_result))
 
   elif (expected_outcome !=
         ExpectedSplitOutcome.MUST_BE_CONSISTENT_IF_SUCCEEDS):
-    raise ValueError('Unknown type of expected outcome: %r',
+    raise ValueError('Unknown type of expected outcome: %r'%
                      expected_outcome)
   current_items.extend([value for value in reader_iter])
 

Reply via email to