Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

pythonPackages.awkward1: init at 0.1.28 #73212

Merged
merged 1 commit into from Dec 8, 2019
Merged

Conversation

@veprbl
Copy link
Member

veprbl commented Nov 11, 2019

Motivation for this change

Implement package awkward 1.0 development branch
https://github.com/scikit-hep/awkward-1.0

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • 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 nix-review --run "nix-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.
Notify maintainers

cc @costrouc

@veprbl veprbl requested review from FRidh and jonringer as code owners Nov 11, 2019
Copy link
Member

FRidh left a comment

Is this needed in Nixpkgs? We have the rule to have only one version of a package in python-packages.nix to prevent potential collisions.

@veprbl

This comment has been minimized.

Copy link
Member Author

veprbl commented Nov 11, 2019

@GrahamcOfBorg build python27Packages.awkward_1 python37Packages.awkward_1 python38Packages.awkward_1

@FRidh There won't be a collision, the new package provides only "import awkward1", the old one will provide "import awkward" and soon "import awkward0".

@jonringer

This comment has been minimized.

Copy link
Contributor

jonringer commented Nov 11, 2019

do you mind having the directories in python-modules be named awkward0 and awkward1. Then it seems much more like two separate packages

@veprbl

This comment has been minimized.

Copy link
Member Author

veprbl commented Nov 11, 2019

do you mind having the directories in python-modules be named awkward0 and awkward1

I don't see a reason to do that. In general, we do this directory structure for most simple multi-version packages. The awkward{0,1} business is just a quirk due to limitations of python, it will be eventually gone.

@veprbl

This comment has been minimized.

Copy link
Member Author

veprbl commented Nov 11, 2019

On the other hand, a following check:

grep -Er '[a-z]_[0-9]+ = callPackage'  pkgs/top-level/python-packages.nix 
grep -Er '[a-z][0-9]+ = callPackage'  pkgs/top-level/python-packages.nix

seems to indicate that if we want to maintain consistency, I should be instead renaming the attributes fromawkward_{0,1} to awkward{0,1} despite the different convention currently preferred in the all-packages.nix

@veprbl veprbl force-pushed the veprbl:pr/awkward1_init branch from 8fa6474 to bcbc56e Nov 11, 2019
@veprbl veprbl changed the title pythonPackages.awkward_1: init at 0.1.19 pythonPackages.awkward1: init at 0.1.20 Nov 11, 2019
@veprbl

This comment has been minimized.

Copy link
Member Author

veprbl commented Nov 11, 2019

@jonringer Did as you suggested, plus the attribute rename.

@ofborg ofborg bot requested a review from costrouc Nov 11, 2019
@veprbl veprbl force-pushed the veprbl:pr/awkward1_init branch from bcbc56e to 38d7e3b Nov 12, 2019
@veprbl

This comment has been minimized.

Copy link
Member Author

veprbl commented Nov 12, 2019

@GrahamcOfBorg build python27Packages.awkward1 python37Packages.awkward1 python38Packages.awkward1

@FRidh

This comment has been minimized.

Copy link
Member

FRidh commented Nov 14, 2019

@veprbl the package should be named as it is named on PyPI, in normalized form. Checking on PyOI, I see awkward and awkward1 for the names of the packages. This is what the attributes should be called.

We suffix packages with e.g. _1 if we include part of the version of the package in it. But here there is no need, because the two packages are simply named differently already.

@veprbl

This comment has been minimized.

Copy link
Member Author

veprbl commented Nov 14, 2019

@FRidh Thank you for clarifying. Should I then remove the commit that creates the awkward0 attribute? My idea was to try to future-proof this, but the convention where attributes correspond to what PyPi provides also makes a sense to me.

@FRidh

This comment has been minimized.

Copy link
Member

FRidh commented Nov 14, 2019

Yes, please remove it.

@veprbl veprbl force-pushed the veprbl:pr/awkward1_init branch from 38d7e3b to 17e5c46 Nov 14, 2019
@veprbl veprbl changed the title pythonPackages.awkward1: init at 0.1.20 pythonPackages.awkward1: init at 0.1.22 Nov 14, 2019
@jonringer

This comment has been minimized.

Copy link
Contributor

jonringer commented Nov 15, 2019

I agree with matching the name conventions on pypi, adheres to the law of least surprise when someone is trying to use it.

@veprbl veprbl force-pushed the veprbl:pr/awkward1_init branch from 17e5c46 to 8ade12e Dec 8, 2019
@veprbl veprbl changed the title pythonPackages.awkward1: init at 0.1.22 pythonPackages.awkward1: init at 0.1.28 Dec 8, 2019
@veprbl

This comment has been minimized.

Copy link
Member Author

veprbl commented Dec 8, 2019

@GrahamcOfBorg build python27Packages.awkward1 python37Packages.awkward1 python38Packages.awkward1

@veprbl veprbl force-pushed the veprbl:pr/awkward1_init branch from 8ade12e to cc23348 Dec 8, 2019
@veprbl veprbl merged commit 8a08418 into NixOS:master Dec 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.