gensim: added Gensim 2.1.0 and related tools#25804
gensim: added Gensim 2.1.0 and related tools#25804aptlin wants to merge 7 commits intoNixOS:masterfrom
Conversation
FRidh
left a comment
There was a problem hiding this comment.
Looks good. Some minor changes are needed. I haven't tested the PR though.
There was a problem hiding this comment.
Note that currently this is the default checkPhase so its not necessary. No need to change it though.
pkgs/top-level/python-packages.nix
Outdated
There was a problem hiding this comment.
even though the order of the attributes in this file is a mess, do try to stick to it.
There was a problem hiding this comment.
@FRidh, do you want me to order them alphabetically? They are in the order of the commits. Unfortunately, I did not know all the missing dependencies at the beginning, so I propose to leave the order as it is now.
There was a problem hiding this comment.
Try to find for each attribute an appropriate place in the quasi-alphabetically ordered attributes
There was a problem hiding this comment.
What is the point, @FRidh, if they are all related to one package -- gensim? Spreading them over the entire file would only increase the entropy, it seems.
|
Thank you very much for the review, @FRidh, I have updated the files with the changes requested. |
|
Oh, I now noticed one aspect that has to be fixed. Your expressions are in files named after the package. Instead, the rule is to put them in a folder, named after the package. |
|
But there are other packages, like numpy, which are not in their own folder -- should they also be, in principle, moved to their own folders? |
Yes, they should. |
|
@FRidh, I have moved all the recipes related to the PR. |
|
@FRidh, Travis CI on OS X seems to fail due to the following error: Do you think we should try to fix it by specifying I have also updated the recipe for smart_open, which build failed because it's dependency, boto, is tailored to Python2 and decided to try to fix it by replacing boto with boto3. I have also changed the doCheck boolean for selectors in the recipy for pyro4. |
61aba52 to
277f1f5
Compare
eb9486f to
3b44a28
Compare
|
@FRidh, the problem with importing urllib2 persists in the Travis CI build of boto for GNU/Linux (I am not sure why boto and boto3 are built). Shall I disable checking of boto in python3 with |
|
@sdll just ignore Travis, we don't rely on it. Currently I get $ nix-build -A python.pkgs.gensim
...
/build/gensim-2.1.0/dist /build/gensim-2.1.0
Processing ./gensim-2.1.0-cp27-cp27mu-linux_x86_64.whl
Requirement already satisfied: scipy>=0.7.0 in /nix/store/xg1v4g2hzcvj7raakaf1gvxhsq3l95rb-python2.7-scipy-0.18.1/lib/python2.7/site-packages (from gensim==2.1.0)
Requirement already satisfied: six>=1.5.0 in /nix/store/m0apm5s8w43q8r9q9ciz3xj6vvxj5xci-python2.7-six-1.10.0/lib/python2.7/site-packages (from gensim==2.1.0)
Requirement already satisfied: smart-open>=1.2.1 in /nix/store/0l6jv4ip60gk8wvydsiki4a3ys09jr46-python2.7-smart_open-1.5.2/lib/python2.7/site-packages (from gensim==2.1.0)
Requirement already satisfied: numpy>=1.3 in /nix/store/2r7iirb4h95rnvvf4cns7471gkibnpsv-python2.7-numpy-1.11.3/lib/python2.7/site-packages (from gensim==2.1.0)
Collecting boto>=2.32 (from smart-open>=1.2.1->gensim==2.1.0)
Could not find a version that satisfies the requirement boto>=2.32 (from smart-open>=1.2.1->gensim==2.1.0) (from versions: )
No matching distribution found for boto>=2.32 (from smart-open>=1.2.1->gensim==2.1.0)
builder for ‘/nix/store/6md6n0vqc9r58zfm8zq7p6d6am1349n0-python2.7-gensim-2.1.0.drv’ failed with exit code 1
error: build of ‘/nix/store/6md6n0vqc9r58zfm8zq7p6d6am1349n0-python2.7-gensim-2.1.0.drv’ failed
|
|
@FRidh, so we need to use boto then, not boto3, sorry for the confusion -- I checked only if smart_open builds fine. I have reverted to boto. Could you please try again? |
|
@FRidh Is there anything preventing this from being merged? I just rebased this and built gensim locally with no issues. |
|
I've done some overlapping changes in PR #26525. Perhaps you can rebase your work on master? |
|
Closing because #26525 was merged. If there are other changes you would like to get in, please open a new PR. |
Motivation for this change
Making NLP analysis easier with Nix.
Things done
(nix.useSandbox on NixOS,
or option
build-use-sandboxinnix.confon non-NixOS)
nix-shell -p nox --run "nox-review wip"./result/bin/)