-
Notifications
You must be signed in to change notification settings - Fork 4.5k
[BEAM-3731] Enable tests to run in Python 3 #4730
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
54b59ee
f497023
34eb5c6
cc38e6a
4475bc7
7a06b88
39d4337
8b1795d
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 |
|---|---|---|
|
|
@@ -25,6 +25,8 @@ | |
| import time | ||
| import uuid | ||
|
|
||
| import six | ||
|
|
||
| from apache_beam.internal import util | ||
| from apache_beam.io import iobase | ||
| from apache_beam.io.filesystem import BeamIOError | ||
|
|
@@ -73,10 +75,10 @@ def __init__(self, | |
| ~exceptions.ValueError: if **shard_name_template** is not of expected | ||
| format. | ||
| """ | ||
| if not isinstance(file_path_prefix, (basestring, ValueProvider)): | ||
| if not isinstance(file_path_prefix, (six.string_types, ValueProvider)): | ||
|
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. Another option is to just use
Contributor
Author
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. I think this would be good. Are we ok with adding future as a dependency to master? Or should we get a separate Python3 branch first.
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. I think future is a reasonable dependency to take in master. Although the idea of a Python 3 branch could be good if we continue to take a long time to review/merge Python 3 fixes into master (although then we'd need people to be able to cooperate on a Python 3 branch explicitly). |
||
| raise TypeError('file_path_prefix must be a string or ValueProvider;' | ||
| 'got %r instead' % file_path_prefix) | ||
| if not isinstance(file_name_suffix, (basestring, ValueProvider)): | ||
| if not isinstance(file_name_suffix, (six.string_types, ValueProvider)): | ||
| raise TypeError('file_name_suffix must be a string or ValueProvider;' | ||
| 'got %r instead' % file_name_suffix) | ||
|
|
||
|
|
@@ -87,9 +89,9 @@ def __init__(self, | |
| shard_name_template = DEFAULT_SHARD_NAME_TEMPLATE | ||
| elif shard_name_template == '': | ||
| num_shards = 1 | ||
| if isinstance(file_path_prefix, basestring): | ||
| if isinstance(file_path_prefix, six.string_types): | ||
| file_path_prefix = StaticValueProvider(str, file_path_prefix) | ||
| if isinstance(file_name_suffix, basestring): | ||
| if isinstance(file_name_suffix, six.string_types): | ||
| file_name_suffix = StaticValueProvider(str, file_name_suffix) | ||
| self.file_path_prefix = file_path_prefix | ||
| self.file_name_suffix = file_name_suffix | ||
|
|
@@ -221,7 +223,7 @@ def _rename_batch(batch): | |
| except BeamIOError as exp: | ||
| if exp.exception_details is None: | ||
| raise | ||
| for (src, dest), exception in exp.exception_details.iteritems(): | ||
| for (src, dest), exception in exp.exception_details.items(): | ||
| if exception: | ||
| logging.warning('Rename not successful: %s -> %s, %s', src, dest, | ||
| exception) | ||
|
|
@@ -243,7 +245,7 @@ def _rename_batch(batch): | |
| return exceptions | ||
|
|
||
| exception_batches = util.run_using_threadpool( | ||
| _rename_batch, zip(source_file_batch, destination_file_batch), | ||
| _rename_batch, list(zip(source_file_batch, destination_file_batch)), | ||
| num_threads) | ||
|
|
||
| all_exceptions = [e for exception_batch in exception_batches | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -26,7 +26,9 @@ | |
| :class:`~apache_beam.io._AvroSource`. | ||
| """ | ||
|
|
||
| from six import integer_types | ||
|
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. Would it maybe make sense to import integer_types and string_types with the from directive here (or I guess what is the reason for the change)?
Contributor
Author
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. Either way works. I just chose one way to keep things consistent after applying the auto-converter. Is there anything I'm missing? |
||
| from __future__ import absolute_import | ||
|
|
||
| import six | ||
|
|
||
| from apache_beam.internal import pickler | ||
| from apache_beam.io import concat_source | ||
|
|
@@ -98,12 +100,12 @@ def __init__(self, | |
| result. | ||
| """ | ||
|
|
||
| if not isinstance(file_pattern, (basestring, ValueProvider)): | ||
| if not isinstance(file_pattern, (six.string_types, 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, basestring): | ||
| if isinstance(file_pattern, six.string_types): | ||
| file_pattern = StaticValueProvider(str, file_pattern) | ||
| self._pattern = file_pattern | ||
|
|
||
|
|
@@ -234,11 +236,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, six.integer_types): | ||
| 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, six.integer_types): | ||
| 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.
It maybe makes sense to add an explicit byte test in coders_test