Grant Henke 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 ( Without it tests fail with: ``` > builder.add_column('decimal_val', type_=kudu.decimal, precision=5, > scale=2) E AttributeError: 'module' object has no attribute 'decimal' ``` 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. Done 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 i Done 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? Done 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. Done -- 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: Grant Henke <[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 21:21:07 +0000 Gerrit-HasComments: Yes
