Cleanup pkgs/top-level/python-packages.nix #27379

wizzup · 2017-07-14T10:28:00Z

Motivation for this change

This is the first batch of python-packages.nix cleanup as discuss here.

Things done

Tested using sandboxing : nix.useSandbox on NixOS
Built on platform(s)
- NixOS
- macOS
- Linux
Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
Tested execution : tested using this scripts
Fits CONTRIBUTING.md.

Additional information

altair and pygame_sdl2 failed to build on python 3.6 before this PR, I test against upstream/master

Try to run following command on your system, if there is no error please reply to me so I can fix them.

$ nix-shell -p python36Packages.altair
$ nix-shell -p python36Packages.pygame_sdl2

Mic92 · 2017-07-14T10:36:35Z

pkgs/development/python-modules/django_tagging/default.nix

+buildPythonPackage rec {
+  name = "django-tagging-0.4.5";
+
+  src = fetchurl {


do you also want to convert instances of fetchurl to fetchPypi or do you want to keep this out of scope of pull request?

I think for now, to make it easy for me, just moving buildPythonPackges out and use callPackage first.

I recommend making several changes at the same time. Most importantly is using

fetchPypi when possible

have pname, version and name attributes so we can more easily update packages

Example https://github.com/NixOS/nixpkgs/blob/master/pkgs/development/python-modules/jupyter_client/default.nix

wizzup · 2017-07-14T10:41:56Z

@FRidh This 0d16766 is ok with nix-shell -p

$ nix-shell -I nixpkgs=/nix/nixpkgs -p pythonPackages.django_tagging

[nix-shell:/nix/nixpkgs]$

but failed on nox-review.

$ nix-shell -p nox --run 'nox-review wip'                                                                                                                                                              
error: while evaluating anonymous function at /nix/nixpkgs/lib/meta.nix:47:54, called from /nix/nixpkgs/lib/attrsets.nix:199:52:                                                                       
while evaluating ‘isDerivation’ at /nix/nixpkgs/lib/attrsets.nix:295:18, called from /nix/nixpkgs/lib/meta.nix:47:62:                                                                                  
while evaluating the attribute ‘django_tagging_0_3’ at /nix/nixpkgs/pkgs/top-level/python-packages.nix:9943:3:                                                                                         
while evaluating ‘override’ at /nix/nixpkgs/lib/customisation.nix:78:20, called from /nix/nixpkgs/pkgs/top-level/python-packages.nix:9943:24:                                                          
while evaluating ‘makeOverridable’ at /nix/nixpkgs/lib/customisation.nix:72:24, called from /nix/nixpkgs/lib/customisation.nix:78:29:                                                                  
anonymous function at /nix/nixpkgs/pkgs/development/python-modules/django_tagging/default.nix:1:1 called with unexpected argument ‘name’, at /nix/nixpkgs/lib/customisation.nix:74:12                  
Traceback (most recent call last):                                                                                                                                                                     
  File "/nix/store/kdfayij6fxnmgdly9l9zyl0np6wgp4gc-nox-0.0.5/bin/.nox-review-wrapped", line 12, in <module>                                                                                           
    sys.exit(cli())                                                                                                                                                                                    
  File "/nix/store/nn3d0x4q24ba5h42b1lqrxxf9m3p1gb5-python3.6-click-6.7/lib/python3.6/site-packages/click/core.py", line 722, in __call__                                                              
    return self.main(*args, **kwargs)                                                                                                                                                                  
  File "/nix/store/nn3d0x4q24ba5h42b1lqrxxf9m3p1gb5-python3.6-click-6.7/lib/python3.6/site-packages/click/core.py", line 697, in main                                                                  
    rv = self.invoke(ctx)                                                                                                                                                                              
  File "/nix/store/nn3d0x4q24ba5h42b1lqrxxf9m3p1gb5-python3.6-click-6.7/lib/python3.6/site-packages/click/core.py", line 1066, in invoke                                                               
    return _process_result(sub_ctx.command.invoke(sub_ctx))                                                                                                                                            
  File "/nix/store/nn3d0x4q24ba5h42b1lqrxxf9m3p1gb5-python3.6-click-6.7/lib/python3.6/site-packages/click/core.py", line 895, in invoke                                                                
    return ctx.invoke(self.callback, **ctx.params)                                                                                                                                                     
  File "/nix/store/nn3d0x4q24ba5h42b1lqrxxf9m3p1gb5-python3.6-click-6.7/lib/python3.6/site-packages/click/core.py", line 535, in invoke                                                                
    return callback(*args, **kwargs)                                                                                                                                                                   
  File "/nix/store/nn3d0x4q24ba5h42b1lqrxxf9m3p1gb5-python3.6-click-6.7/lib/python3.6/site-packages/click/decorators.py", line 17, in new_func                                                         
    return f(get_current_context(), *args, **kwargs)                                                                                                                                                   
  File "/nix/store/kdfayij6fxnmgdly9l9zyl0np6wgp4gc-nox-0.0.5/lib/python3.6/site-packages/nox/review.py", line 82, in wip                                                                              
    packages('.'))                                                                                                                                                                                     
  File "/nix/store/kdfayij6fxnmgdly9l9zyl0np6wgp4gc-nox-0.0.5/lib/python3.6/site-packages/nox/nixpkgs_repo.py", line 81, in packages                                                                   
    universal_newlines=True)                                                                                                                                                                           
  File "/nix/store/v96ayp4hp476fqabmx86vgvwcn8wb7f9-python3-3.6.1/lib/python3.6/subprocess.py", line 336, in check_output                                                                              
    **kwargs).stdout                                                                                                                                                                                   
  File "/nix/store/v96ayp4hp476fqabmx86vgvwcn8wb7f9-python3-3.6.1/lib/python3.6/subprocess.py", line 418, in run                                                                                       
    output=stdout, stderr=stderr)                                                                                                                                                                      
subprocess.CalledProcessError: Command '['nix-env', '-f', '.', '-qaP', '--out-path', '--show-trace']' returned non-zero exit status 1.

Is it OK?

Mic92 · 2017-07-14T10:44:49Z

pkgs/top-level/python-packages.nix

-      homepage = https://github.com/Fantomas42/django-tagging;
-    };
-  };
+  django_tagging = callPackage ../development/python-modules/django_tagging { };

  django_tagging_0_3 = self.django_tagging.override (attrs: rec {


This should be probably changed to

django_tagging_0_3 = self.django_tagging.overrideDerivation (attrs: rec {

Good point, but it should be overrideAttrs then since we want to override the call to stdenv.mkDerivation and not to derivation.

This package can be also removed at some point, when graphite-web is upgraded:

https://github.com/graphite-project/graphite-web/blob/master/requirements.txt

bjornfor · 2017-07-14T13:08:46Z

@wizzup: Please add proper commit messages, i.e. "package: what"

wizzup · 2017-07-14T13:11:31Z

@bjornfor Oh, I thought I will git rebase -i or maybe squash all commits after reviewed.

There are no change to the packages itself, just cleanup work.

bjornfor · 2017-07-14T13:13:54Z

@wizzup: Ok. Just don't forget :-)

FRidh · 2017-07-14T13:48:21Z

@wizzup just don't pass in pkgs or pythonPackages but use individual packages as function parameters.

wizzup · 2017-07-14T14:38:19Z

@FRidh I don't think I understand what you mean, could you point to code or show me some example?

I have to go for now, will continue later (tomorrow maybe).

Mic92 · 2017-07-14T16:54:44Z

pkgs/development/python-modules/discid/default.nix

@@ -0,0 +1,27 @@
+{
+  stdenv, lib, pkgs,


@FRidh meant, instead of using pkgs here, use libdiscid here.

Mic92 · 2017-07-14T16:55:23Z

pkgs/development/python-modules/discid/default.nix

+
+  patchPhase = ''
+    substituteInPlace discid/libdiscid.py \
+      --replace '_open_library(_LIB_NAME)' "_open_library('${pkgs.libdiscid}/lib/libdiscid.so.0')"


then you would write:

substituteInPlace discid/libdiscid.py \ --replace '_open_library(_LIB_NAME)' "_open_library('${libdiscid}/lib/libdiscid.so.0')"

It's funny, I remember I have try this and it failed so I have to put pkgs back in.
Let my try again.

Mic92 · 2017-07-14T16:56:37Z

pkgs/development/python-modules/pygame_sdl2/default.nix

@@ -0,0 +1,34 @@
+{
+stdenv, lib, pkgs, buildPythonPackage, fetchFromGitHub,


pkgs is not used here

This one I do really forget to remove.

wizzup · 2017-07-15T08:05:12Z

pkgs/development/python-modules/pypandoc/default.nix

+{
+stdenv, buildPythonPackage, fetchPypi,
+pip, pandoc, glibcLocales,
+haskellPackages, texlive


@FRidh How can I not use haskellPacakges and texlive here (for line 27)? reference to this contex

@wizzup for haskellPackages and texlive your approach is ok.

Mic92 · 2017-07-15T10:26:17Z

I would recommend to not add more python packages to this pull request. Otherwise it will be difficult to find reviewers and the risc of merge conflicts increases.

wizzup · 2017-07-15T10:34:10Z

Should I create new PR for another batch?

Do I have to squash all commit in to single one with proper commit message?

wizzup · 2017-07-15T10:37:48Z

I use these script to test my code

git diff upstream/master --summary | cut -d '/' -f 4 - | xargs -i ./tstpy.sh {} 2>/dev/null

tstpy.sh

NAME=$1

echo ${NAME}

nix-shell --run 'echo 2.7 Done' -I nixpkgs=/nix/nixpkgs -p python27Packages.${NAME}
nix-shell --run 'echo 3.6 Done' -I nixpkgs=/nix/nixpkgs -p python36Packages.${NAME}

Mic92 · 2017-07-15T10:53:09Z

I would propose the following approach:

Leave the commits as they are, but keep them a more descriptive commit title like:

python.pkgs.$name: move to separate expression

You can probably also automated this (https://davidwalsh.name/update-git-commit-messages)
Then mark the pull request as non-WIP and wait for it to merge.

This is a tedious task, but unless we have to tools to automate moving out python expressions,
we need to ensure to not breaking something accidentally.

move to separate expression

Mic92 · 2017-07-15T16:42:59Z

I changed the commit title (there was a newline between package name and remaining message)
and I removed lib from the individual packages.

wizzup · 2017-07-16T05:36:20Z

Cool! Sorry about newline in commit message. I never use filter-branch before.

For the next batch I will use proper commit message from start.

FRidh · 2017-07-16T09:43:14Z

Ah, I thought only one commit had been pushed. Closing again.
Thanks to you both for the "boring" work! :)

Mic92 reviewed Jul 14, 2017

View reviewed changes

wizzup force-pushed the fix-pythonPackages branch 3 times, most recently from e27febd to f7214a2 Compare July 14, 2017 12:04

Mic92 reviewed Jul 14, 2017

View reviewed changes

wizzup force-pushed the fix-pythonPackages branch from 786bc4e to 4c4ee53 Compare July 15, 2017 07:52

wizzup commented Jul 15, 2017

View reviewed changes

python.pkgs.django_tagging

071e095

move to separate expression

wizzup force-pushed the fix-pythonPackages branch from 8209c79 to ffd56ac Compare July 15, 2017 12:47

wizzup added 9 commits July 15, 2017 20:14

python-packages.nix cleanup : overrideAttrs

dbd7ed5

python.pkgs.acoustics

38b5d69

move to separate expression

python.pkgs.ansicolor

55664cf

move to separate expression

python.pkgs.emcee

b953bac

move to separate expression

python.pkgs.discid

e890258

move to separate expression

python.pkgs.pygame_sdl2

290a646

move to separate expression

python-packages.nix cleanup : remove pkgs

a9ab6fd

python.pkgs.unifi

1a8fbb9

move to separate expression

python.pkgs.aafigure

5bb3bb0

move to separate expression

wizzup added 13 commits July 15, 2017 20:16

python.pkgs.altair

aecbece

move to separate expression

python.pkgs.vega

0fa5f70

move to separate expression

python.pkgs.acme

7046583

move to separate expression

python.pkgs.zeroconf

3b8b9fe

move to separate expression

python.pkgs.vine

8f24381

move to separate expression

python.pkgs.stripe

8803796

move to separate expression

python.pkgs.yamllint

a840b2e

move to separate expression

python.pkgs.pypandoc

603082b

move to separate expression

python.pkgs.pytoml

c27bc8c

move to separate expression

python.pkgs.packaging

6a11a25

move to separate expression

python.pkgs.intervaltree

abb200f

move to separate expression

python.pkgs.jsonref

5f94a0a

move to separate expression

python.pkgs.yapf

b39ae73

move to separate expression

wizzup force-pushed the fix-pythonPackages branch from ffd56ac to b39ae73 Compare July 15, 2017 13:19

wizzup changed the title ~~[WIP] cleanup pkgs/top-level/python-packages.nix~~ Cleanup pkgs/top-level/python-packages.nix Jul 15, 2017

Mic92 closed this in a5d75f0 Jul 15, 2017

FRidh reopened this Jul 16, 2017

FRidh closed this Jul 16, 2017

wizzup deleted the fix-pythonPackages branch July 19, 2017 08:14

wizzup mentioned this pull request Jul 19, 2017

Cleanup pkgs/top-level/python-packages.nix #27497

Merged

6 tasks

wizzup mentioned this pull request Jul 28, 2017

Cleanup pkgs/top-level/python-packages.nix #27706

Merged

6 tasks

wizzup mentioned this pull request Aug 31, 2017

Cleanup pkgs/top-level/python-packages.nix #28793

Merged

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Cleanup pkgs/top-level/python-packages.nix #27379

Cleanup pkgs/top-level/python-packages.nix #27379

wizzup commented Jul 14, 2017 •

edited

Mic92 Jul 14, 2017

wizzup Jul 14, 2017

FRidh Jul 14, 2017 •

edited

wizzup commented Jul 14, 2017

Mic92 Jul 14, 2017

FRidh Jul 14, 2017

Mic92 Jul 14, 2017

bjornfor commented Jul 14, 2017

wizzup commented Jul 14, 2017 •

edited

bjornfor commented Jul 14, 2017

FRidh commented Jul 14, 2017

wizzup commented Jul 14, 2017

Mic92 Jul 14, 2017

Mic92 Jul 14, 2017

wizzup Jul 15, 2017

Mic92 Jul 14, 2017

wizzup Jul 15, 2017

wizzup Jul 15, 2017

Mic92 Jul 15, 2017

Mic92 commented Jul 15, 2017 •

edited

wizzup commented Jul 15, 2017

wizzup commented Jul 15, 2017

Mic92 commented Jul 15, 2017

Mic92 commented Jul 15, 2017

wizzup commented Jul 16, 2017

FRidh commented Jul 16, 2017

		@@ -0,0 +1,34 @@
		{
		stdenv, lib, pkgs, buildPythonPackage, fetchFromGitHub,

Cleanup pkgs/top-level/python-packages.nix #27379

Cleanup pkgs/top-level/python-packages.nix #27379

Conversation

wizzup commented Jul 14, 2017 • edited

Motivation for this change

Things done

Additional information

Choose a reason for hiding this comment

Choose a reason for hiding this comment

FRidh Jul 14, 2017 • edited

Choose a reason for hiding this comment

wizzup commented Jul 14, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bjornfor commented Jul 14, 2017

wizzup commented Jul 14, 2017 • edited

bjornfor commented Jul 14, 2017

FRidh commented Jul 14, 2017

wizzup commented Jul 14, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Mic92 commented Jul 15, 2017 • edited

wizzup commented Jul 15, 2017

wizzup commented Jul 15, 2017

Mic92 commented Jul 15, 2017

Mic92 commented Jul 15, 2017

wizzup commented Jul 16, 2017

FRidh commented Jul 16, 2017

wizzup commented Jul 14, 2017 •

edited

FRidh Jul 14, 2017 •

edited

wizzup commented Jul 14, 2017 •

edited

Mic92 commented Jul 15, 2017 •

edited