Bug 1265609: Fix pandas not getting installed

[dinhxuanvu]

Pandas package fails to install properly if it's included in setup.py for
Python 2 & 3 applications. This issue is possibly due to a bug with numpy
dependency of pandas package. This commit allows pandas package to be
installed using 'pip install -e .' instead of 'python setup.py install'
to work around the numpy bug.

Bug <1265609>
Link https://bugzilla.redhat.com/show_bug.cgi?id=1265609

Signed-off-by: Vu Dinh vdinh@redhat.com

[dinhxuanvu]

@tiwillia @Miciah @dobbymoodge Please review when you have a chance. Thanks. [test]

[dinhxuanvu]

@tiwillia @Miciah @dobbymoodge Please review when you have a chance. Thanks. [test]

[openshift-bot]

Online Test Results: SUCCESS (https://ci.dev.openshift.redhat.com/jenkins/job/test_pull_requests/9000/)

[openshift-bot]

Online Test Results: SUCCESS (https://ci.dev.openshift.redhat.com/jenkins/job/test_pull_requests/9000/)

[dinhxuanvu]

@soltysh @rhcarvalho Would you guys mind talking a look at this bug and its fix as well? I would really appreciate the consultation on this.

Here are my findings and explanation (excerpt from a sent email) regarding the bug:

I attach the copies (see [A] & [B]) of command execution verbose from both "python setup.py install" and the new "pip install -e .". Also, here is the link [1] to the unknown numpy dependency bug that is probably associated with the pandas bug.

[A] http://pastebin.test.redhat.com/323063
[B] http://pastebin.test.redhat.com/323066
[1] numpy/numpy#2434
[2] http://python-packaging-user-guide.readthedocs.org/en/latest/pip_easy_install/
[3] http://stackoverflow.com/questions/3220404/why-use-pip-over-easy-install

Essentually, 'pip install' option uses the pip install itself to install three main dependencies (including numpy) of the pandas package (please refer to [A] & [B] documents) while 'python setup.py install' is using setuptools (easy_install) I think (see [2]). It's simply better at dependency-resolving and that's all it is as far as I can see. Python install looks like it just goes straight to install pandas package without carefully verifying the dependencies associated it and end up failing. The known bug with numpy listed above is possibly contributing to the error as well.

In my very own point of view, pip install option is safe as I have tested with several popular packages such as django and it is fine. In fact, the pip install option is preferable option as I found several online conversations pointing its advantages (see [3]). We can simply replace the python install with pip install completely if necessary.

Thanks in advance!

[dinhxuanvu]

@soltysh @rhcarvalho Would you guys mind talking a look at this bug and its fix as well? I would really appreciate the consultation on this.

Here are my findings and explanation (excerpt from a sent email) regarding the bug:

I attach the copies (see [A] & [B]) of command execution verbose from both "python setup.py install" and the new "pip install -e .". Also, here is the link [1] to the unknown numpy dependency bug that is probably associated with the pandas bug.

[A] http://pastebin.test.redhat.com/323063
[B] http://pastebin.test.redhat.com/323066
[1] numpy/numpy#2434
[2] http://python-packaging-user-guide.readthedocs.org/en/latest/pip_easy_install/
[3] http://stackoverflow.com/questions/3220404/why-use-pip-over-easy-install

Essentually, 'pip install' option uses the pip install itself to install three main dependencies (including numpy) of the pandas package (please refer to [A] & [B] documents) while 'python setup.py install' is using setuptools (easy_install) I think (see [2]). It's simply better at dependency-resolving and that's all it is as far as I can see. Python install looks like it just goes straight to install pandas package without carefully verifying the dependencies associated it and end up failing. The known bug with numpy listed above is possibly contributing to the error as well.

In my very own point of view, pip install option is safe as I have tested with several popular packages such as django and it is fine. In fact, the pip install option is preferable option as I found several online conversations pointing its advantages (see [3]). We can simply replace the python install with pip install completely if necessary.

Thanks in advance!

[soltysh]

You probably want to pass $OPENSHIFT_PYTHON_MIRROR $OPENSHIFT_PIP_TRUSTED_HOST like we do that a couple lines before.

[soltysh]

One nit, and the change itself LGTM.

As for the transition, from my research, it's perfectly ok to make the transition from python setup.py develop to pip install -e .. Even more, it's highly encouraged to do that. Additionally we are already using pip install -r requirements.txt couple lines before so this will unify the entire install process.
The only question is do we want such a big change? @danmcp

[soltysh]

One nit, and the change itself LGTM.

As for the transition, from my research, it's perfectly ok to make the transition from python setup.py develop to pip install -e .. Even more, it's highly encouraged to do that. Additionally we are already using pip install -r requirements.txt couple lines before so this will unify the entire install process.
The only question is do we want such a big change? @danmcp

[dinhxuanvu]

@soltysh Thanks for the review and consultation. Certainly appreciated it. I agreed with the moving to pip install since it's newer and more reliable (as you said we already use it). If @danmcp is fine with this, I will just use pip install as default and remove the conditional statement + python install in the code as well.

[dinhxuanvu]

@soltysh Thanks for the review and consultation. Certainly appreciated it. I agreed with the moving to pip install since it's newer and more reliable (as you said we already use it). If @danmcp is fine with this, I will just use pip install as default and remove the conditional statement + python install in the code as well.

[danmcp]

@soltysh @dinhxuanvu I am only ok with this in general if we are 100% confident in no regressions for existing apps. It seems like a big change.

[danmcp]

@soltysh @dinhxuanvu I am only ok with this in general if we are 100% confident in no regressions for existing apps. It seems like a big change.

[dinhxuanvu]

@tiwillia @abhgupta

[dinhxuanvu]

@tiwillia @abhgupta

[abhgupta]

@soltysh @danmcp The reason we are considering going down the path of using pip for installing these dependencies was that python setup.py install was failing for at least one package. Additionally, we didn't want to make a one-off change in the code that hard-coded the specific package (pandas).

An alternative approach (not sure if its required or recommended), would be to always use python setup.py install first and if it fails then fall back to pip install. Does that make sense?

[abhgupta]

@soltysh @danmcp The reason we are considering going down the path of using pip for installing these dependencies was that python setup.py install was failing for at least one package. Additionally, we didn't want to make a one-off change in the code that hard-coded the specific package (pandas).

An alternative approach (not sure if its required or recommended), would be to always use python setup.py install first and if it fails then fall back to pip install. Does that make sense?

[danmcp]

@abhgupta I think I would prefer an environment variable to let people choose to use pip and the default stay the same.

[danmcp]

@abhgupta I think I would prefer an environment variable to let people choose to use pip and the default stay the same.

[abhgupta]

@danmcp I would prefer that as well. Keep python setup.py install as the default that can be overridden by the user env var. Keeping the existing install option as default will ensure we don't break existing applications.

[abhgupta]

@danmcp I would prefer that as well. Keep python setup.py install as the default that can be overridden by the user env var. Keeping the existing install option as default will ensure we don't break existing applications.

[abhgupta]

@dinhxuanvu I believe we have settled on an approach

[abhgupta]

@dinhxuanvu I believe we have settled on an approach

[soltysh]

👍

[soltysh]

👍

[dinhxuanvu]

@abhgupta @tiwillia I have changed the code to follow the approach that we discussed above. Please review when you can. [test]

[dinhxuanvu]

@abhgupta @tiwillia I have changed the code to follow the approach that we discussed above. Please review when you can. [test]

[soltysh]

A nit, but can you fix the indentation - it reads better 😉

[soltysh]

LGTM. Don't forget to update the docs with info about the marker!

[soltysh]

LGTM. Don't forget to update the docs with info about the marker!

[dinhxuanvu]

@soltysh Thanks for the comment. Fixed and [test]

[dinhxuanvu]

@soltysh Thanks for the comment. Fixed and [test]

[openshift-bot]

Evaluated for online test up to 9d05226

[openshift-bot]

Evaluated for online test up to 9d05226

[abhgupta]

[merge]

[abhgupta]

[merge]

[openshift-bot]

Evaluated for online merge up to 9d05226

[openshift-bot]

Evaluated for online merge up to 9d05226

[dinhxuanvu]

@tiwillia The merge fails and it looks like an EC2 timeout error. Would you mind taking a look to verify that it's not related to my changes and re-merge it if possible? Thanks

[dinhxuanvu]

@tiwillia The merge fails and it looks like an EC2 timeout error. Would you mind taking a look to verify that it's not related to my changes and re-merge it if possible? Thanks

[openshift-bot]

Online Merge Results: SUCCESS (https://ci.dev.openshift.redhat.com/jenkins/job/merge_pull_requests/6607/) (Image: devenv_5690)

[openshift-bot]

Online Merge Results: SUCCESS (https://ci.dev.openshift.redhat.com/jenkins/job/merge_pull_requests/6607/) (Image: devenv_5690)