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 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

Copy link
Member

@FRidh FRidh left a comment

Choose a reason for hiding this comment

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

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
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
Copy link
Contributor

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

@veprbl
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
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 changed the title pythonPackages.awkward_1: init at 0.1.19 pythonPackages.awkward1: init at 0.1.20 Nov 11, 2019
@veprbl
Copy link
Member Author

veprbl commented Nov 11, 2019

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

@veprbl
Copy link
Member Author

veprbl commented Nov 12, 2019

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

@FRidh
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
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
Copy link
Member

FRidh commented Nov 14, 2019

Yes, please remove it.

@veprbl veprbl changed the title pythonPackages.awkward1: init at 0.1.20 pythonPackages.awkward1: init at 0.1.22 Nov 14, 2019
@jonringer
Copy link
Contributor

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 changed the title pythonPackages.awkward1: init at 0.1.22 pythonPackages.awkward1: init at 0.1.28 Dec 8, 2019
@veprbl
Copy link
Member Author

veprbl commented Dec 8, 2019

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

@veprbl veprbl merged commit 8a08418 into NixOS:master Dec 8, 2019
@veprbl veprbl deleted the pr/awkward1_init branch December 1, 2020 17:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants