Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/9496 )
Change subject: KUDU-721: [Python] Add DECIMAL column type support ...................................................................... Patch Set 1: (5 comments) http://gerrit.cloudera.org:8080/#/c/9496/1/python/kudu/__init__.py File python/kudu/__init__.py: http://gerrit.cloudera.org:8080/#/c/9496/1/python/kudu/__init__.py@39 PS1, Line 39: unixtime_micros, bool_ as bool, decimal, Do you know what's the purpose of these large import statements? Very few (perhaps none?) of them are referenced here. http://gerrit.cloudera.org:8080/#/c/9496/1/python/kudu/schema.pyx File python/kudu/schema.pyx: http://gerrit.cloudera.org:8080/#/c/9496/1/python/kudu/schema.pyx@367 PS1, Line 367: Clients can specify a scale for decimal columns. Nit: odd line wrapping here, and on L346 too. http://gerrit.cloudera.org:8080/#/c/9496/1/python/kudu/tests/test_util.py File python/kudu/tests/test_util.py: http://gerrit.cloudera.org:8080/#/c/9496/1/python/kudu/tests/test_util.py@34 PS1, Line 34: def test_to_unscaled_decimal(self): Since you're inheriting from unittest.TestCase, you should be able to use its assertion methods. See https://docs.python.org/2/library/unittest.html#test-cases for details. http://gerrit.cloudera.org:8080/#/c/9496/1/python/kudu/tests/util.py File python/kudu/tests/util.py: http://gerrit.cloudera.org:8080/#/c/9496/1/python/kudu/tests/util.py@20 PS1, Line 20: from decimal import * What is this import needed for here? http://gerrit.cloudera.org:8080/#/c/9496/1/python/kudu/util.py File python/kudu/util.py: http://gerrit.cloudera.org:8080/#/c/9496/1/python/kudu/util.py@20 PS1, Line 20: from decimal import * Can we avoid this? Seems extreme. -- To view, visit http://gerrit.cloudera.org:8080/9496 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8e0855100ab1ea891f990931ec94d0b98c0dece1 Gerrit-Change-Number: 9496 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Jordan Birdsell <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Wes McKinney <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Comment-Date: Mon, 05 Mar 2018 20:40:44 +0000 Gerrit-HasComments: Yes
