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.transformers: init at 2.2.1 #75106

Merged
merged 5 commits into from Dec 18, 2019
Merged

pythonPackages.transformers: init at 2.2.1 #75106

merged 5 commits into from Dec 18, 2019

Conversation

pashashocky
Copy link
Contributor

@pashashocky pashashocky commented Dec 6, 2019

Adding the transformers library for DeepLearning along side the dependencies to build it.

Motivation for this change

Wanted to build the transformers library for ML work.

This is my first PR into NixOS, so please let me know if I am doing something wrong and I will try to rectify this in the future, but I struggled quite a bit through this - so happy that it builds. This builds on my system and made sure this builds with Sandboxing.

Wasn't sure if I had to create multiple PR's for each separate package that transformers depends on, but it's all in here.

Tests for transformers exist, but i don't think they will work in sandboxing because they require downloading packages from s3, and also require to have tensorflow or pytorch installed... I will try to get them to work one more time, but failed on first try.

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 @jonringer @teh @FRidh - Not sure who I am supposed to be tagging for PR's that don't have any maintainers.

@pashashocky pashashocky changed the title transformers: init at 2.2.1 pythonPackages.transformers: init at 2.2.1 Dec 6, 2019
maintainers/maintainer-list.nix Show resolved Hide resolved
pkgs/development/libraries/sentencepiece/default.nix Outdated Show resolved Hide resolved
pkgs/development/python-modules/sacremoses/default.nix Outdated Show resolved Hide resolved
pkgs/development/python-modules/sentencepiece/default.nix Outdated Show resolved Hide resolved
pkgs/development/python-modules/sentencepiece/default.nix Outdated Show resolved Hide resolved
pkgs/development/python-modules/transformers/default.nix Outdated Show resolved Hide resolved
pkgs/top-level/python-packages.nix Outdated Show resolved Hide resolved
pkgs/top-level/python-packages.nix Outdated Show resolved Hide resolved
@pashashocky
Copy link
Contributor Author

pashashocky commented Dec 6, 2019

@jonringer - thank you for your comments and pointers, fixed and rebased. Hope it's more consistent now.

@jonringer
Copy link
Contributor

@GrahamcOfBorg build python27Packages.sacremoses python27Packages.sentencepiece python27Packages.transformers python37Packages.sacremoses python37Packages.sentencepiece python37Packages.transformers python38Packages.sacremoses python38Packages.sentencepiece python38Packages.transformers sentencepiece

@jonringer
Copy link
Contributor

do you mind updating the platforms for the packages? I'm trying to not have broken arch or darwin packages get added.

It's fine if this package just doesn't support those platforms

@pashashocky
Copy link
Contributor Author

@jonringer - done, hopefully correctly, once again, appreciate your help!

Copy link
Contributor

@jonringer jonringer left a comment

Choose a reason for hiding this comment

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

diff LGTM
commits LGTM

[14 built, 4 copied (8.8 MiB), 1.4 MiB DL]
https://github.com/NixOS/nixpkgs/pull/75106
10 package were built:
python27Packages.sacremoses python27Packages.sentencepiece python27Packages.transformers python37Packages.sacremoses python37Packages.sentencepiece python37Packages.transformers python38Packages.sacremoses python38Packages.sentencepiece python38Packages.transformers sentencepiece

@jonringer
Copy link
Contributor

@GrahamcOfBorg build python27Packages.sacremoses python27Packages.sentencepiece python27Packages.transformers python37Packages.sacremoses python37Packages.sentencepiece python37Packages.transformers python38Packages.sacremoses python38Packages.sentencepiece python38Packages.transformers sentencepiece

@jonringer
Copy link
Contributor

been busy with real life + oni, sorry for not getting back to this faster

@jonringer
Copy link
Contributor

@GrahamcOfBorg build python27Packages.sacremoses python27Packages.sentencepiece python27Packages.transformers python37Packages.sacremoses python37Packages.sentencepiece python37Packages.transformers python38Packages.sacremoses python38Packages.sentencepiece python38Packages.transformers sentencepiece

@jonringer
Copy link
Contributor

platformo issues resolved

@jonringer jonringer merged commit dae6544 into NixOS:master Dec 18, 2019
@pashashocky
Copy link
Contributor Author

@jonringer Thank you for your help! I learned a lot!

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