[email protected] has posted comments on this change. Change subject: KUDU-1691 - [python] Replaced group varint encoding with bitshuffle ......................................................................
Patch Set 2: (1 comment) Apologies, I just saw the earlier comments. Will address over the weekend. http://gerrit.cloudera.org:8080/#/c/4824/2/python/kudu/tests/test_schema.py File python/kudu/tests/test_schema.py: Line 131: del foo > I don't think you need to delete foo, do you? also could you make sure the I deleted foo to make sure that there is no variable persistence in the for loop. For example, running for i in range(5): a = i + 1 print(a) will print 5 even though the value is outside of the for loop. I added the del foo to make sure that the previous value is not assessed during the assertion. I can remove the del if you think it is appropriate to. My view is that it would probably be best to remove it once the encoding is asserted explicitly. I was unable to check the encoding of the column using the below methods: 1. foo.encoding returns <function ColumnSpec.encoding> but does not specify the value of the encoding. 2. bulding the schema and accessing the column (using schema['foo_auto']) does not have an encoding property. That is, once the encoding it set, we can identify only whether or not it has been set, not what it was set to. I looked at extracting the value of the encoding explicitly from C++ however my Cython/C++ is not up to that level yet. -- To view, visit http://gerrit.cloudera.org:8080/4824 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I18a611e746d48879d6e9988abcee42e0354dcfdc Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: [email protected] Gerrit-Reviewer: David Ribeiro Alves <[email protected]> Gerrit-Reviewer: Jordan Birdsell <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-HasComments: Yes
