FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Unpin MxNet by lopuhin · Pull Request #169 · Kaggle/docker-python · GitHub

  1. Kaggle
  2. docker-python

Repository navigation

Unpin MxNet - #169

Merged
nerdcha merged 2 commits into
Kaggle:masterfrom
lopuhin:unpin-mxnet
Jan 28, 2018
ConversationCommits (2)ChecksFiles changed
Merged

Unpin MxNet#169
nerdcha merged 2 commits into
Kaggle:masterfrom
lopuhin:unpin-mxnet

Conversation

lopuhin commented Jan 24, 2018

Copy link
Copy Markdown
Contributor

This unpins mxnet version. It was build from source before, and then moved to pip install in 6c4cac1.

I verified that package is installed correctly by building the docker image locally. Here is the relevant part of the build log where the package is installed:

Collecting mxnet
  Downloading mxnet-1.0.0.post4-py2.py3-none-manylinux1_x86_64.whl (27.4MB)
Requirement already satisfied: requests==2.18.4 in /opt/conda/lib/python3.6/site-packages (from mxnet)
Collecting graphviz==0.8.1 (from mxnet)
  Downloading graphviz-0.8.1-py2.py3-none-any.whl
Requirement already satisfied: numpy<=1.13.3 in /opt/conda/lib/python3.6/site-packages (from mxnet)
Requirement already satisfied: chardet<3.1.0,>=3.0.2 in /opt/conda/lib/python3.6/site-packages (from requests==2.18.4->mxnet)
Requirement already satisfied: idna<2.7,>=2.5 in /opt/conda/lib/python3.6/site-packages (from requests==2.18.4->mxnet)
Requirement already satisfied: urllib3<1.23,>=1.21.1 in /opt/conda/lib/python3.6/site-packages (from requests==2.18.4->mxnet)
Requirement already satisfied: certifi>=2017.4.17 in /opt/conda/lib/python3.6/site-packages (from requests==2.18.4->mxnet)
Installing collected packages: graphviz, mxnet
Successfully installed graphviz-0.8.1 mxnet-1.0.0.post4

and other major libraries seems to be fine as well.

sebbov reviewed Jan 25, 2018
View reviewed changes

sebbov left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Thanks Konstantin!

Anything you could also contribute to test mxnet invariants (even if just importing the right module(s)) in https://github.com/Kaggle/docker-python/blob/master/test_build.py ?

Otherwise this lgtm, but I'll let @nerdcha approve since he may be aware of a reason for the existing pinning.

sebbov requested a review from nerdcha January 25, 2018 18:28

lopuhin commented Jan 25, 2018

Copy link
Copy Markdown
Contributor Author

Thanks @sebbov , I'll rebase on master and double-check that mxnet is still installed correctly, and will add a check to test_build.py.

Unpin mxnet version
efbc01e 
This unpins mxnet version. It was build from source before, and then moved to pip install in 6c4cac1.
Add mxnet and mxnet.gluon tests
ea34e64

lopuhin commented Jan 25, 2018

Copy link
Copy Markdown
Contributor Author

@nerdcha double-checked that everything is ok, rebased on master and added check that mxnet and mxnet.gluon can be imported to test_build.py.

nerdcha commented Jan 28, 2018

Copy link
Copy Markdown
Contributor

I'd pinned the version because I was following the practice from MXNet's docs, no other reason. This is much better. Thanks very much!

nerdcha merged commit fbb04f9 into Kaggle:master Jan 28, 2018
lopuhin deleted the unpin-mxnet branch January 29, 2018 06:58
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

nerdcha Awaiting requested review from nerdcha
1 more reviewer
sebbov sebbov left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

Back | FazBrowse Home | New Git URL