pythonPackages.phik: init at 0.9.8 #71248
Conversation
| meta = with lib; { | ||
| description = "Phi_K is a new and practical correlation coefficient based on several refinements to Pearson’s hypothesis test of independence of two variables."; | ||
| homepage = https://phik.readthedocs.io/en/latest/; | ||
| maintainers = with lib.maintainers; [ melsigl ]; |
There was a problem hiding this comment.
You should probably specify the license: license = lib.licenses.asl20;
There was a problem hiding this comment.
Or license = licenses.asl20 because of the with lib; above.
| meta = with lib; { | ||
| description = "Phi_K is a new and practical correlation coefficient based on several refinements to Pearson’s hypothesis test of independence of two variables."; | ||
| homepage = https://phik.readthedocs.io/en/latest/; | ||
| maintainers = with lib.maintainers; [ melsigl ]; |
There was a problem hiding this comment.
Or license = licenses.asl20 because of the with lib; above.
| }; | ||
|
|
||
| propagatedBuildInputs = [ | ||
| nbconvert |
There was a problem hiding this comment.
These can probably be made buildInputs instead of propagatedBuildInputs. Really I don't think they should be needed at all, since they're seem to be just for the tests which aren't being run, but I think there is a mistake in the setup.py (I think this line shouldn't exist: https://github.com/KaveIO/PhiK/blob/v0.9.8/setup.py#L47) which makes them necessary. Maybe also add jupyter_client and pytest as buildInputs.
Maybe someone more experienced has opinions on whether the 'mistake' I mentioned should be patched out/fixed upstream, or if it's fine as is?
There was a problem hiding this comment.
Ah yes, #71248 (comment) works so put them (nbconvert and pytest-pylint) in checkInputs, not buildInputs. I'm still convinced the setup.py is mistaken, but it shouldn't matter too much.
| phases = [ "unpackPhase" "installPhase" ]; | ||
|
|
||
| meta = with lib; { | ||
| description = "Phi_K is a new and practical correlation coefficient based on several refinements to Pearson’s hypothesis test of independence of two variables."; |
There was a problem hiding this comment.
This is quite a long description so should probably be the longDescription, and the description can just be "Phi_K correlation analyzer library"
| pytest-pylint | ||
| ]; | ||
|
|
||
| buildInputs = [ |
There was a problem hiding this comment.
I got a bit muddled in my comment that suggested making these buildInputs - these two should be checkInputs actually.
There was a problem hiding this comment.
Should be indeed buildInputs.
melsigl
force-pushed
the
master
branch
2 times, most recently
from
8cdef48
to
a3b53db
Compare
|
Actually, one more thing: the to remove it. |
|
|
There was a problem hiding this comment.
nix-review passes on NixOS
diff LGTM
outputs LGTM
[2 built (4 failed), 1 copied (0.0 MiB), 0.0 MiB DL]
error: build of '/nix/store/n3yxi36yqgp6inwdd11rw7072axfqibd-env.drv' failed
https://github.com/NixOS/nixpkgs/pull/71248
1 package failed to build:
python38Packages.phik
1 package were build:
python37Packages.phik
[nix-shell:/home/jon/.cache/nix-review/pr-71248]$ python
Python 3.7.4 (default, Jul 8 2019, 18:31:06)
[GCC 8.3.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import phik
>>>
Motivation for this change
Phi_K is a new and practical correlation coefficient based on several refinements to Pearson’s hypothesis test of independence of two variables.
Things done
sandboxinnix.confon non-NixOS)nix-shell -p nix-review --run "nix-review wip"./result/bin/)nix path-info -Sbefore and after)