-
Notifications
You must be signed in to change notification settings - Fork 4.5k
[BEAM-4000] Futurize io subpackage #5715
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -26,8 +26,10 @@ | |
| :class:`~apache_beam.io._AvroSource`. | ||
| """ | ||
|
|
||
| from six import integer_types | ||
| from six import string_types | ||
| from __future__ import absolute_import | ||
|
|
||
| from past.builtins import long | ||
| from past.builtins import unicode | ||
|
|
||
| from apache_beam.internal import pickler | ||
| from apache_beam.io import concat_source | ||
|
|
@@ -99,12 +101,12 @@ def __init__(self, | |
| result. | ||
| """ | ||
|
|
||
| if not isinstance(file_pattern, (string_types, ValueProvider)): | ||
| if not isinstance(file_pattern, ((str, unicode), ValueProvider)): | ||
| raise TypeError('%s: file_pattern must be of type string' | ||
| ' or ValueProvider; got %r instead' | ||
| % (self.__class__.__name__, file_pattern)) | ||
|
|
||
| if isinstance(file_pattern, string_types): | ||
| if isinstance(file_pattern, (str, unicode)): | ||
| file_pattern = StaticValueProvider(str, file_pattern) | ||
| self._pattern = file_pattern | ||
|
|
||
|
|
@@ -235,11 +237,11 @@ class _SingleFileSource(iobase.BoundedSource): | |
|
|
||
| def __init__(self, file_based_source, file_name, start_offset, stop_offset, | ||
| min_bundle_size=0, splittable=True): | ||
| if not isinstance(start_offset, integer_types): | ||
| if not isinstance(start_offset, (int, long)): | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If you import from See http://python-future.org/compatible_idioms.html#long-integers
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If you want to replace this with just Instead, we can replace the try/except block with from |
||
| 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, integer_types): | ||
| if not isinstance(stop_offset, (int, long)): | ||
| raise TypeError( | ||
| 'stop_offset must be a number. Received: %r' % stop_offset) | ||
| if start_offset >= stop_offset: | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IIUC run_using_threadpool accepts any iterable, so forcing a list is redundant.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reading
util.run_using_threadpool, it looks like the code callslen(inputs), which may not be compatible with all iterables.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ack, please discard my initial comment.