[jira] [Work logged] (BEAM-4008) Futurize and fix python 2 compatibility for utils subpackage
[ https://issues.apache.org/jira/browse/BEAM-4008?focusedWorklogId=111591=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-111591 ] ASF GitHub Bot logged work on BEAM-4008: Author: ASF GitHub Bot Created on: 13/Jun/18 18:16 Start Date: 13/Jun/18 18:16 Worklog Time Spent: 10m Work Description: aaltay closed pull request #5336: [BEAM-4008] Futurize utils subpackage URL: https://github.com/apache/beam/pull/5336 This is a PR merged from a forked repository. As GitHub hides the original diff on merge, it is displayed below for the sake of provenance: As this is a foreign pull request (from a fork), the diff is supplied below (as it won't show otherwise due to GitHub magic): diff --git a/sdks/python/apache_beam/utils/__init__.py b/sdks/python/apache_beam/utils/__init__.py index 635c80f7c6b..5bc12e7e282 100644 --- a/sdks/python/apache_beam/utils/__init__.py +++ b/sdks/python/apache_beam/utils/__init__.py @@ -19,3 +19,5 @@ For internal use only; no backwards-compatibility guarantees. """ + +from __future__ import absolute_import diff --git a/sdks/python/apache_beam/utils/annotations.py b/sdks/python/apache_beam/utils/annotations.py index 036b08287df..6f62ce93116 100644 --- a/sdks/python/apache_beam/utils/annotations.py +++ b/sdks/python/apache_beam/utils/annotations.py @@ -60,6 +60,8 @@ def exp_multiply(arg1, arg2): print exp_multiply(5,6) """ +from __future__ import absolute_import + import warnings from functools import partial from functools import wraps diff --git a/sdks/python/apache_beam/utils/annotations_test.py b/sdks/python/apache_beam/utils/annotations_test.py index ddd1b9ff757..2901e3b3632 100644 --- a/sdks/python/apache_beam/utils/annotations_test.py +++ b/sdks/python/apache_beam/utils/annotations_test.py @@ -15,6 +15,8 @@ # limitations under the License. # +from __future__ import absolute_import + import unittest import warnings diff --git a/sdks/python/apache_beam/utils/counters.py b/sdks/python/apache_beam/utils/counters.py index 46ac8ff723a..5696bc43f80 100644 --- a/sdks/python/apache_beam/utils/counters.py +++ b/sdks/python/apache_beam/utils/counters.py @@ -23,7 +23,11 @@ For internal use only; no backwards-compatibility guarantees. """ +from __future__ import absolute_import + import threading +from builtins import hex +from builtins import object from collections import namedtuple from apache_beam.transforms import cy_combiners diff --git a/sdks/python/apache_beam/utils/plugin.py b/sdks/python/apache_beam/utils/plugin.py index 563b93c54c7..1425874ed3b 100644 --- a/sdks/python/apache_beam/utils/plugin.py +++ b/sdks/python/apache_beam/utils/plugin.py @@ -20,6 +20,10 @@ For experimental usage only; no backwards-compatibility guarantees. """ +from __future__ import absolute_import + +from builtins import object + class BeamPlugin(object): """Plugin base class to be extended by dependent users such as FileSystem. diff --git a/sdks/python/apache_beam/utils/processes.py b/sdks/python/apache_beam/utils/processes.py index e5fd9c84aab..b0e8e3c8ba5 100644 --- a/sdks/python/apache_beam/utils/processes.py +++ b/sdks/python/apache_beam/utils/processes.py @@ -20,6 +20,8 @@ For internal use only; no backwards-compatibility guarantees. """ +from __future__ import absolute_import + import platform import subprocess diff --git a/sdks/python/apache_beam/utils/processes_test.py b/sdks/python/apache_beam/utils/processes_test.py index 2dd45f44dc5..123c124adc4 100644 --- a/sdks/python/apache_beam/utils/processes_test.py +++ b/sdks/python/apache_beam/utils/processes_test.py @@ -16,6 +16,8 @@ # """Unit tests for the processes module.""" +from __future__ import absolute_import + import unittest import mock diff --git a/sdks/python/apache_beam/utils/profiler.py b/sdks/python/apache_beam/utils/profiler.py index 9f9c8cd1629..18a712fff64 100644 --- a/sdks/python/apache_beam/utils/profiler.py +++ b/sdks/python/apache_beam/utils/profiler.py @@ -20,21 +20,19 @@ For internal use only; no backwards-compatibility guarantees. """ -import cProfile +from __future__ import absolute_import + +import cProfile # pylint: disable=bad-python3-import +import io import logging import os import pstats -import sys import tempfile import time import warnings +from builtins import object from threading import Timer -if sys.version_info[0] < 3: - import StringIO -else: - from io import StringIO - class Profile(object): """cProfile wrapper context for saving and logging profiler results.""" @@ -71,7 +69,7 @@ def __exit__(self, *args): os.remove(filename) if self.log_results: - s = StringIO() + s = io.StringIO() self.stats = pstats.Stats( self.profile, stream=s).sort_stats(Profile.SORTBY) self.stats.print_stats() diff --git a/sdks/python/apache_beam/utils/proto_utils.py
[jira] [Work logged] (BEAM-4008) Futurize and fix python 2 compatibility for utils subpackage
[ https://issues.apache.org/jira/browse/BEAM-4008?focusedWorklogId=111426=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-111426 ] ASF GitHub Bot logged work on BEAM-4008: Author: ASF GitHub Bot Created on: 13/Jun/18 07:34 Start Date: 13/Jun/18 07:34 Worklog Time Spent: 10m Work Description: tvalentyn commented on issue #5336: [BEAM-4008] Futurize utils subpackage URL: https://github.com/apache/beam/pull/5336#issuecomment-396843049 LGTM, thank you, @RobbeSneyders! This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 111426) Time Spent: 2h 10m (was: 2h) > Futurize and fix python 2 compatibility for utils subpackage > > > Key: BEAM-4008 > URL: https://issues.apache.org/jira/browse/BEAM-4008 > Project: Beam > Issue Type: Sub-task > Components: sdk-py-core >Reporter: Robbe >Assignee: Robbe >Priority: Major > Time Spent: 2h 10m > Remaining Estimate: 0h > -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (BEAM-4008) Futurize and fix python 2 compatibility for utils subpackage
[ https://issues.apache.org/jira/browse/BEAM-4008?focusedWorklogId=111425=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-111425 ] ASF GitHub Bot logged work on BEAM-4008: Author: ASF GitHub Bot Created on: 13/Jun/18 07:33 Start Date: 13/Jun/18 07:33 Worklog Time Spent: 10m Work Description: tvalentyn commented on a change in pull request #5336: [BEAM-4008] Futurize utils subpackage URL: https://github.com/apache/beam/pull/5336#discussion_r194981003 ## File path: sdks/python/apache_beam/utils/windowed_value.py ## @@ -178,34 +182,22 @@ def __repr__(self): self.windows, self.pane_info) + def _key(self): +return self.value, self.timestamp_micros, self.windows, self.pane_info + + def __eq__(self, other): +return (type(self) == type(other) +and self.timestamp_micros == other.timestamp_micros +and self.value == self.value Review comment: No worries, thank you. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 111425) Time Spent: 2h (was: 1h 50m) > Futurize and fix python 2 compatibility for utils subpackage > > > Key: BEAM-4008 > URL: https://issues.apache.org/jira/browse/BEAM-4008 > Project: Beam > Issue Type: Sub-task > Components: sdk-py-core >Reporter: Robbe >Assignee: Robbe >Priority: Major > Time Spent: 2h > Remaining Estimate: 0h > -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (BEAM-4008) Futurize and fix python 2 compatibility for utils subpackage
[ https://issues.apache.org/jira/browse/BEAM-4008?focusedWorklogId=111410=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-111410 ] ASF GitHub Bot logged work on BEAM-4008: Author: ASF GitHub Bot Created on: 13/Jun/18 06:32 Start Date: 13/Jun/18 06:32 Worklog Time Spent: 10m Work Description: RobbeSneyders commented on a change in pull request #5336: [BEAM-4008] Futurize utils subpackage URL: https://github.com/apache/beam/pull/5336#discussion_r194968867 ## File path: sdks/python/apache_beam/utils/windowed_value.py ## @@ -178,34 +182,22 @@ def __repr__(self): self.windows, self.pane_info) + def _key(self): +return self.value, self.timestamp_micros, self.windows, self.pane_info + + def __eq__(self, other): +return (type(self) == type(other) +and self.timestamp_micros == other.timestamp_micros +and self.value == self.value Review comment: Sorry, took this straight from the benchmark and didn't notice it. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 111410) Time Spent: 1h 50m (was: 1h 40m) > Futurize and fix python 2 compatibility for utils subpackage > > > Key: BEAM-4008 > URL: https://issues.apache.org/jira/browse/BEAM-4008 > Project: Beam > Issue Type: Sub-task > Components: sdk-py-core >Reporter: Robbe >Assignee: Robbe >Priority: Major > Time Spent: 1h 50m > Remaining Estimate: 0h > -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (BEAM-4008) Futurize and fix python 2 compatibility for utils subpackage
[ https://issues.apache.org/jira/browse/BEAM-4008?focusedWorklogId=111394=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-111394 ] ASF GitHub Bot logged work on BEAM-4008: Author: ASF GitHub Bot Created on: 13/Jun/18 04:02 Start Date: 13/Jun/18 04:02 Worklog Time Spent: 10m Work Description: tvalentyn commented on a change in pull request #5336: [BEAM-4008] Futurize utils subpackage URL: https://github.com/apache/beam/pull/5336#discussion_r194950717 ## File path: sdks/python/apache_beam/utils/windowed_value.py ## @@ -178,34 +182,22 @@ def __repr__(self): self.windows, self.pane_info) + def _key(self): +return self.value, self.timestamp_micros, self.windows, self.pane_info + + def __eq__(self, other): +return (type(self) == type(other) +and self.timestamp_micros == other.timestamp_micros +and self.value == self.value Review comment: s/self/other in right hand side of the equality. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 111394) Time Spent: 1h 40m (was: 1.5h) > Futurize and fix python 2 compatibility for utils subpackage > > > Key: BEAM-4008 > URL: https://issues.apache.org/jira/browse/BEAM-4008 > Project: Beam > Issue Type: Sub-task > Components: sdk-py-core >Reporter: Robbe >Assignee: Robbe >Priority: Major > Time Spent: 1h 40m > Remaining Estimate: 0h > -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (BEAM-4008) Futurize and fix python 2 compatibility for utils subpackage
[ https://issues.apache.org/jira/browse/BEAM-4008?focusedWorklogId=111395=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-111395 ] ASF GitHub Bot logged work on BEAM-4008: Author: ASF GitHub Bot Created on: 13/Jun/18 04:02 Start Date: 13/Jun/18 04:02 Worklog Time Spent: 10m Work Description: tvalentyn commented on a change in pull request #5336: [BEAM-4008] Futurize utils subpackage URL: https://github.com/apache/beam/pull/5336#discussion_r194950463 ## File path: sdks/python/apache_beam/utils/windowed_value.py ## @@ -178,34 +182,22 @@ def __repr__(self): self.windows, self.pane_info) + def _key(self): Review comment: We don't need this anymore. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 111395) Time Spent: 1h 40m (was: 1.5h) > Futurize and fix python 2 compatibility for utils subpackage > > > Key: BEAM-4008 > URL: https://issues.apache.org/jira/browse/BEAM-4008 > Project: Beam > Issue Type: Sub-task > Components: sdk-py-core >Reporter: Robbe >Assignee: Robbe >Priority: Major > Time Spent: 1h 40m > Remaining Estimate: 0h > -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (BEAM-4008) Futurize and fix python 2 compatibility for utils subpackage
[ https://issues.apache.org/jira/browse/BEAM-4008?focusedWorklogId=111265=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-111265 ] ASF GitHub Bot logged work on BEAM-4008: Author: ASF GitHub Bot Created on: 12/Jun/18 20:34 Start Date: 12/Jun/18 20:34 Worklog Time Spent: 10m Work Description: RobbeSneyders commented on a change in pull request #5336: [BEAM-4008] Futurize utils subpackage URL: https://github.com/apache/beam/pull/5336#discussion_r194879406 ## File path: sdks/python/apache_beam/utils/windowed_value.py ## @@ -178,33 +182,14 @@ def __repr__(self): self.windows, self.pane_info) - def __hash__(self): -return (hash(self.value) + -3 * self.timestamp_micros + -7 * hash(self.windows) + -11 * hash(self.pane_info)) - - # We'd rather implement __eq__, but Cython supports that via __richcmp__ - # instead. Fortunately __cmp__ is understood by both (but not by Python 3). - def __cmp__(left, right): # pylint: disable=no-self-argument -"""Compares left and right for equality. - -For performance reasons, doesn't actually impose an ordering -on unequal values (always returning 1). -""" -if type(left) is not type(right): - return cmp(type(left), type(right)) + def _key(self): +return self.value, self.timestamp_micros, self.windows, self.pane_info -# TODO(robertwb): Avoid the type checks? -# Returns False (0) if equal, and True (1) if not. -return not WindowedValue._typed_eq(left, right) + def __eq__(self, other): +return type(self) == type(other) and self._key() == other._key() - @staticmethod - def _typed_eq(left, right): -return (left.timestamp_micros == right.timestamp_micros -and left.value == right.value -and left.windows == right.windows -and left.pane_info == right.pane_info) + def __hash__(self): Review comment: Thanks for adding the benchmarking, this is very useful! I just ran the benchmark myself and got a performance regression of 6% for reading and 15% for adding to a dict. I will revert the changes. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 111265) Time Spent: 1.5h (was: 1h 20m) > Futurize and fix python 2 compatibility for utils subpackage > > > Key: BEAM-4008 > URL: https://issues.apache.org/jira/browse/BEAM-4008 > Project: Beam > Issue Type: Sub-task > Components: sdk-py-core >Reporter: Robbe >Assignee: Robbe >Priority: Major > Time Spent: 1.5h > Remaining Estimate: 0h > -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (BEAM-4008) Futurize and fix python 2 compatibility for utils subpackage
[ https://issues.apache.org/jira/browse/BEAM-4008?focusedWorklogId=110940=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-110940 ] ASF GitHub Bot logged work on BEAM-4008: Author: ASF GitHub Bot Created on: 12/Jun/18 03:35 Start Date: 12/Jun/18 03:35 Worklog Time Spent: 10m Work Description: tvalentyn commented on a change in pull request #5336: [BEAM-4008] Futurize utils subpackage URL: https://github.com/apache/beam/pull/5336#discussion_r194606891 ## File path: sdks/python/apache_beam/utils/windowed_value.py ## @@ -178,33 +182,14 @@ def __repr__(self): self.windows, self.pane_info) - def __hash__(self): -return (hash(self.value) + -3 * self.timestamp_micros + -7 * hash(self.windows) + -11 * hash(self.pane_info)) - - # We'd rather implement __eq__, but Cython supports that via __richcmp__ - # instead. Fortunately __cmp__ is understood by both (but not by Python 3). - def __cmp__(left, right): # pylint: disable=no-self-argument -"""Compares left and right for equality. - -For performance reasons, doesn't actually impose an ordering -on unequal values (always returning 1). -""" -if type(left) is not type(right): - return cmp(type(left), type(right)) + def _key(self): +return self.value, self.timestamp_micros, self.windows, self.pane_info -# TODO(robertwb): Avoid the type checks? -# Returns False (0) if equal, and True (1) if not. -return not WindowedValue._typed_eq(left, right) + def __eq__(self, other): +return type(self) == type(other) and self._key() == other._key() - @staticmethod - def _typed_eq(left, right): -return (left.timestamp_micros == right.timestamp_micros -and left.value == right.value -and left.windows == right.windows -and left.pane_info == right.pane_info) + def __hash__(self): Review comment: Thanks for your patience for awaiting the feedback, @RobbeSneyders. I doublechecked performance of __hash__ using a microbenchmark. Previous implementation of __hash__ appears to be at least 7.5% faster, possibly due to overheads to create a tuple, and/or the way Cython compiles it. I suggest we revert to previous implementation of __hash__. For consistency we can also implement __eq__ by comparing fields directly. My microbenchmark can be found in https://github.com/apache/beam/compare/master...tvalentyn:utils_futurization_benchmark, see https://github.com/tvalentyn/beam/commit/67d3df0f1248f43feb84503361f0071efc560fc2. It depends on the mini-framework that's being finalized in https://github.com/apache/beam/pull/5565. CC: @robertwb This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 110940) Time Spent: 1h 20m (was: 1h 10m) > Futurize and fix python 2 compatibility for utils subpackage > > > Key: BEAM-4008 > URL: https://issues.apache.org/jira/browse/BEAM-4008 > Project: Beam > Issue Type: Sub-task > Components: sdk-py-core >Reporter: Robbe >Assignee: Robbe >Priority: Major > Time Spent: 1h 20m > Remaining Estimate: 0h > -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (BEAM-4008) Futurize and fix python 2 compatibility for utils subpackage
[ https://issues.apache.org/jira/browse/BEAM-4008?focusedWorklogId=106330=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-106330 ] ASF GitHub Bot logged work on BEAM-4008: Author: ASF GitHub Bot Created on: 28/May/18 13:23 Start Date: 28/May/18 13:23 Worklog Time Spent: 10m Work Description: RobbeSneyders commented on a change in pull request #5336: [BEAM-4008] Futurize utils subpackage URL: https://github.com/apache/beam/pull/5336#discussion_r191205722 ## File path: sdks/python/apache_beam/utils/profiler.py ## @@ -20,21 +20,19 @@ For internal use only; no backwards-compatibility guarantees. """ -import cProfile +from __future__ import absolute_import + +import cProfile # pylint: disable=bad-python3-import Review comment: This is a bug in pylint. See https://github.com/PyCQA/pylint/issues/1612 This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 106330) Time Spent: 1h 10m (was: 1h) > Futurize and fix python 2 compatibility for utils subpackage > > > Key: BEAM-4008 > URL: https://issues.apache.org/jira/browse/BEAM-4008 > Project: Beam > Issue Type: Sub-task > Components: sdk-py-core >Reporter: Robbe >Assignee: Robbe >Priority: Major > Time Spent: 1h 10m > Remaining Estimate: 0h > -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (BEAM-4008) Futurize and fix python 2 compatibility for utils subpackage
[ https://issues.apache.org/jira/browse/BEAM-4008?focusedWorklogId=105398=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-105398 ] ASF GitHub Bot logged work on BEAM-4008: Author: ASF GitHub Bot Created on: 23/May/18 23:00 Start Date: 23/May/18 23:00 Worklog Time Spent: 10m Work Description: tvalentyn commented on a change in pull request #5336: [BEAM-4008] Futurize utils subpackage URL: https://github.com/apache/beam/pull/5336#discussion_r190423680 ## File path: sdks/python/apache_beam/utils/windowed_value.py ## @@ -178,33 +182,14 @@ def __repr__(self): self.windows, self.pane_info) - def __hash__(self): -return (hash(self.value) + -3 * self.timestamp_micros + -7 * hash(self.windows) + -11 * hash(self.pane_info)) - - # We'd rather implement __eq__, but Cython supports that via __richcmp__ - # instead. Fortunately __cmp__ is understood by both (but not by Python 3). - def __cmp__(left, right): # pylint: disable=no-self-argument -"""Compares left and right for equality. - -For performance reasons, doesn't actually impose an ordering -on unequal values (always returning 1). -""" -if type(left) is not type(right): - return cmp(type(left), type(right)) + def _key(self): +return self.value, self.timestamp_micros, self.windows, self.pane_info -# TODO(robertwb): Avoid the type checks? -# Returns False (0) if equal, and True (1) if not. -return not WindowedValue._typed_eq(left, right) + def __eq__(self, other): +return type(self) == type(other) and self._key() == other._key() - @staticmethod - def _typed_eq(left, right): -return (left.timestamp_micros == right.timestamp_micros -and left.value == right.value -and left.windows == right.windows -and left.pane_info == right.pane_info) + def __hash__(self): +return hash((type(self),) + self._key()) Review comment: I think hash(self._key()) would be sufficient. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 105398) > Futurize and fix python 2 compatibility for utils subpackage > > > Key: BEAM-4008 > URL: https://issues.apache.org/jira/browse/BEAM-4008 > Project: Beam > Issue Type: Sub-task > Components: sdk-py-core >Reporter: Robbe >Assignee: Robbe >Priority: Major > Time Spent: 1h > Remaining Estimate: 0h > -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (BEAM-4008) Futurize and fix python 2 compatibility for utils subpackage
[ https://issues.apache.org/jira/browse/BEAM-4008?focusedWorklogId=105397=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-105397 ] ASF GitHub Bot logged work on BEAM-4008: Author: ASF GitHub Bot Created on: 23/May/18 23:00 Start Date: 23/May/18 23:00 Worklog Time Spent: 10m Work Description: tvalentyn commented on a change in pull request #5336: [BEAM-4008] Futurize utils subpackage URL: https://github.com/apache/beam/pull/5336#discussion_r190419324 ## File path: sdks/python/apache_beam/utils/timestamp.py ## @@ -143,11 +148,35 @@ def __int__(self): # Note that the returned value may have lost precision. return self.micros // 100 - def __cmp__(self, other): -# Allow comparisons between Duration and Timestamp values. + def __eq__(self, other): Review comment: How about we keep `__eq__` and `__lt__`, remove the rest and decorate the class with `@functools.total_ordering`? This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 105397) Time Spent: 1h (was: 50m) > Futurize and fix python 2 compatibility for utils subpackage > > > Key: BEAM-4008 > URL: https://issues.apache.org/jira/browse/BEAM-4008 > Project: Beam > Issue Type: Sub-task > Components: sdk-py-core >Reporter: Robbe >Assignee: Robbe >Priority: Major > Time Spent: 1h > Remaining Estimate: 0h > -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (BEAM-4008) Futurize and fix python 2 compatibility for utils subpackage
[ https://issues.apache.org/jira/browse/BEAM-4008?focusedWorklogId=105395=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-105395 ] ASF GitHub Bot logged work on BEAM-4008: Author: ASF GitHub Bot Created on: 23/May/18 23:00 Start Date: 23/May/18 23:00 Worklog Time Spent: 10m Work Description: tvalentyn commented on a change in pull request #5336: [BEAM-4008] Futurize utils subpackage URL: https://github.com/apache/beam/pull/5336#discussion_r190419116 ## File path: sdks/python/apache_beam/utils/timestamp.py ## @@ -143,11 +148,35 @@ def __int__(self): # Note that the returned value may have lost precision. return self.micros // 100 - def __cmp__(self, other): -# Allow comparisons between Duration and Timestamp values. Review comment: Let's keep this comment to explain the reason for the casting. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 105395) Time Spent: 40m (was: 0.5h) > Futurize and fix python 2 compatibility for utils subpackage > > > Key: BEAM-4008 > URL: https://issues.apache.org/jira/browse/BEAM-4008 > Project: Beam > Issue Type: Sub-task > Components: sdk-py-core >Reporter: Robbe >Assignee: Robbe >Priority: Major > Time Spent: 40m > Remaining Estimate: 0h > -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (BEAM-4008) Futurize and fix python 2 compatibility for utils subpackage
[ https://issues.apache.org/jira/browse/BEAM-4008?focusedWorklogId=105396=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-105396 ] ASF GitHub Bot logged work on BEAM-4008: Author: ASF GitHub Bot Created on: 23/May/18 23:00 Start Date: 23/May/18 23:00 Worklog Time Spent: 10m Work Description: tvalentyn commented on a change in pull request #5336: [BEAM-4008] Futurize utils subpackage URL: https://github.com/apache/beam/pull/5336#discussion_r190424036 ## File path: sdks/python/apache_beam/utils/profiler.py ## @@ -20,21 +20,19 @@ For internal use only; no backwards-compatibility guarantees. """ -import cProfile +from __future__ import absolute_import + +import cProfile # pylint: disable=bad-python3-import Review comment: For my education, why does py3 lint complain here? This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 105396) Time Spent: 50m (was: 40m) > Futurize and fix python 2 compatibility for utils subpackage > > > Key: BEAM-4008 > URL: https://issues.apache.org/jira/browse/BEAM-4008 > Project: Beam > Issue Type: Sub-task > Components: sdk-py-core >Reporter: Robbe >Assignee: Robbe >Priority: Major > Time Spent: 50m > Remaining Estimate: 0h > -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (BEAM-4008) Futurize and fix python 2 compatibility for utils subpackage
[ https://issues.apache.org/jira/browse/BEAM-4008?focusedWorklogId=101893=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-101893 ] ASF GitHub Bot logged work on BEAM-4008: Author: ASF GitHub Bot Created on: 14/May/18 21:26 Start Date: 14/May/18 21:26 Worklog Time Spent: 10m Work Description: robertwb commented on a change in pull request #5336: [BEAM-4008] Futurize utils subpackage URL: https://github.com/apache/beam/pull/5336#discussion_r188103724 ## File path: sdks/python/apache_beam/utils/windowed_value.py ## @@ -178,33 +182,14 @@ def __repr__(self): self.windows, self.pane_info) - def __hash__(self): -return (hash(self.value) + -3 * self.timestamp_micros + -7 * hash(self.windows) + -11 * hash(self.pane_info)) - - # We'd rather implement __eq__, but Cython supports that via __richcmp__ - # instead. Fortunately __cmp__ is understood by both (but not by Python 3). - def __cmp__(left, right): # pylint: disable=no-self-argument -"""Compares left and right for equality. - -For performance reasons, doesn't actually impose an ordering -on unequal values (always returning 1). -""" -if type(left) is not type(right): - return cmp(type(left), type(right)) + def _key(self): +return self.value, self.timestamp_micros, self.windows, self.pane_info -# TODO(robertwb): Avoid the type checks? -# Returns False (0) if equal, and True (1) if not. -return not WindowedValue._typed_eq(left, right) + def __eq__(self, other): +return type(self) == type(other) and self._key() == other._key() Review comment: __eq__ is a builtin (slots) method that is optimized automatically. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 101893) Time Spent: 0.5h (was: 20m) > Futurize and fix python 2 compatibility for utils subpackage > > > Key: BEAM-4008 > URL: https://issues.apache.org/jira/browse/BEAM-4008 > Project: Beam > Issue Type: Sub-task > Components: sdk-py-core >Reporter: Robbe >Assignee: Robbe >Priority: Major > Time Spent: 0.5h > Remaining Estimate: 0h > -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (BEAM-4008) Futurize and fix python 2 compatibility for utils subpackage
[ https://issues.apache.org/jira/browse/BEAM-4008?focusedWorklogId=101117=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-101117 ] ASF GitHub Bot logged work on BEAM-4008: Author: ASF GitHub Bot Created on: 11/May/18 13:35 Start Date: 11/May/18 13:35 Worklog Time Spent: 10m Work Description: RobbeSneyders commented on a change in pull request #5336: [BEAM-4008] Futurize utils subpackage URL: https://github.com/apache/beam/pull/5336#discussion_r187615168 ## File path: sdks/python/apache_beam/utils/windowed_value.py ## @@ -178,33 +182,14 @@ def __repr__(self): self.windows, self.pane_info) - def __hash__(self): -return (hash(self.value) + -3 * self.timestamp_micros + -7 * hash(self.windows) + -11 * hash(self.pane_info)) - - # We'd rather implement __eq__, but Cython supports that via __richcmp__ - # instead. Fortunately __cmp__ is understood by both (but not by Python 3). - def __cmp__(left, right): # pylint: disable=no-self-argument -"""Compares left and right for equality. - -For performance reasons, doesn't actually impose an ordering -on unequal values (always returning 1). -""" -if type(left) is not type(right): - return cmp(type(left), type(right)) + def _key(self): +return self.value, self.timestamp_micros, self.windows, self.pane_info -# TODO(robertwb): Avoid the type checks? -# Returns False (0) if equal, and True (1) if not. -return not WindowedValue._typed_eq(left, right) + def __eq__(self, other): +return type(self) == type(other) and self._key() == other._key() Review comment: @robertwb should \_\_eq\_\_ be defined in the .pxd file or is this optimized automatically? This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 101117) Time Spent: 20m (was: 10m) > Futurize and fix python 2 compatibility for utils subpackage > > > Key: BEAM-4008 > URL: https://issues.apache.org/jira/browse/BEAM-4008 > Project: Beam > Issue Type: Sub-task > Components: sdk-py-core >Reporter: Robbe >Assignee: Robbe >Priority: Major > Time Spent: 20m > Remaining Estimate: 0h > -- This message was sent by Atlassian JIRA (v7.6.3#76005)
