Adar Dembo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11557 )

Change subject: Fix Pandas Tests in Python 2.6 client
......................................................................


Patch Set 3:

(7 comments)

I guess I don't really understand; requirements.txt is only mandatory on a 
machine where you're building kudu-python. Anyone who runs 'pip install 
kudu-python' isn't going to interact with requirements.txt (or with 
tests_require for that matter), right? Why is it onerous for pandas to be a 
requirement when building kudu-python?

http://gerrit.cloudera.org:8080/#/c/11557/3//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/11557/3//COMMIT_MSG@12
PS3, Line 12: requirements.txt. We will put the pandas dependecny in the jenkins
dependency


http://gerrit.cloudera.org:8080/#/c/11557/3//COMMIT_MSG@13
PS3, Line 13: intial
initial


http://gerrit.cloudera.org:8080/#/c/11557/3/build-support/jenkins/build-and-test.sh
File build-support/jenkins/build-and-test.sh:

http://gerrit.cloudera.org:8080/#/c/11557/3/build-support/jenkins/build-and-test.sh@506
PS3, Line 506:   # We need to install Pandas manually since its not a required 
package but is needed
Nit: it's

Also terminate with a period.

Below too.


http://gerrit.cloudera.org:8080/#/c/11557/3/build-support/jenkins/build-and-test.sh@508
PS3, Line 508:   pip install pandas
Can you pin this to 0.18, which was the last version to support Python 2.6? 
Also please preserve the breadcrumb comment from requirements.txt; it's useful 
for people who want to learn more.

Also, can you add $PIP_INSTALL_FLAGS after 'pip install'? Below too.


http://gerrit.cloudera.org:8080/#/c/11557/3/build-support/jenkins/build-and-test.sh@567
PS3, Line 567:
Nit: leading whitespace here.


http://gerrit.cloudera.org:8080/#/c/11557/3/python/kudu/tests/test_scanner.py
File python/kudu/tests/test_scanner.py:

http://gerrit.cloudera.org:8080/#/c/11557/3/python/kudu/tests/test_scanner.py@385
PS3, Line 385:     @pytest.mark.skipif((not(kudu.CLIENT_SUPPORTS_PANDAS) and
             :                             (not(kudu.CLIENT_SUPPORTS_DECIMAL))),
Can this be formatted like this?

  @pytest.mark.skipif(not (kudu.CLIENT_SUPPORTS_PANDAS) and
                      not (kudu.CLIENT_SUPPORTS_DECIMAL)),


http://gerrit.cloudera.org:8080/#/c/11557/3/python/setup.py
File python/setup.py:

http://gerrit.cloudera.org:8080/#/c/11557/3/python/setup.py@197
PS3, Line 197:     # pytest 3.3 [1], pytest-timeout 1.2.1 [2], and pandas 0.18 
[3] dropped
             :     # support for python 2.6.
             :     #
             :     # 1. https://docs.pytest.org/en/latest/changelog.html#id164
             :     # 2. https://pypi.org/project/pytest-timeout/#id5
             :     # 3. 
https://pandas.pydata.org/pandas-docs/version/0.23.0/whatsnew.html#v0-18-0-march-13-2016
Update this to remove the pandas reference.



--
To view, visit http://gerrit.cloudera.org:8080/11557
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2cce7d529a804e712ccc32fb2774f5d0ee9dbdf8
Gerrit-Change-Number: 11557
Gerrit-PatchSet: 3
Gerrit-Owner: [email protected]
Gerrit-Reviewer: Adar Dembo <[email protected]>
Gerrit-Reviewer: Jordan Birdsell <[email protected]>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Mon, 01 Oct 2018 22:28:08 +0000
Gerrit-HasComments: Yes

Reply via email to