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
python3Packages.gradient: build fix #110565
Conversation
I am ok with removing the old packages that were broken iirc btw. I can't remember why it took so long and as i am on travel i can't check right now. |
Awesome, here's the pull request for the removal: #110576
No worries, I'll leave this pull request open until we decide whether to remove or keep these packages. |
This is a semi-automatic executed nixpkgs-review which is checked by a human on a best effort basis and does not build all packages (e.g. lumo, tensorflow or pytorch). Result of 15 packages built:
The following issues got detected with the above build packages. python37Packages.gradient-utils: Zero tests run by pytest got detected: 'Ran 0 tests in 0.000s'
Zero tests run by pytest got detected: 'Ran 0 tests in 0.000s'
Zero tests run by pytest got detected: 'Ran 0 tests in 0.000s'
|
@SuperSandro2000 @jonringer: before I address the comments: Shall we rather remove the packages (by merging #110576) instead of adding all these new packages (by merging this PR)? |
Adding 3 packages is in the grand scheme of nixpkgs nothing to worry about. If the package is useful we can keep it. Is there any other reason to remove it? |
Great, no reason (I was just worried about how this simple fix blew up a bit). Anyway, I'll go ahead with this PR and close the other one. 👍 |
every package is a small maintenance burden. I would just leave a comment near pname stating that it's no longer strictly needed, and can be safely removed. If it can be maintained through nixpkgs-update (the bot), then I'm fine with keeping it around. |
30405b7
to
e9ba245
Compare
@SuperSandro2000: adding tests will be a significant undertaking. I have refrained from adding them to keep the PR small. Let me know if I should still go ahead and add tests. Other than that the PR is ready to go. |
It would be nice to have more testing but it is not a hard requirement. |
This is a semi-automatic executed nixpkgs-review which is checked by a human on a best effort basis and does not build all packages (e.g. lumo, tensorflow or pytorch). Result of 15 packages built:
The following issues got detected with the above build packages. python37Packages.gradient-utils: Zero tests run by pytest got detected: 'Ran 0 tests in 0.000s'
Zero tests run by pytest got detected: 'Ran 0 tests in 0.000s'
Zero tests run by pytest got detected: 'Ran 0 tests in 0.000s'
|
This is a semi-automatic executed nixpkgs-review which is checked by a human on a best effort basis and does not build all packages (e.g. lumo, tensorflow or pytorch). Result of 15 packages built:
The following issues got detected with the above build packages. python37Packages.gradient-utils: Zero tests run by pytest got detected: 'Ran 0 tests in 0.000s'
Zero tests run by pytest got detected: 'Ran 0 tests in 0.000s'
Zero tests run by pytest got detected: 'Ran 0 tests in 0.000s'
|
e9ba245
to
07c2121
Compare
@SuperSandro2000: I added tests where I could (gradient-utils) and documented why I couldn't add tests to the rest. |
This is a semi-automatic executed nixpkgs-review with nixpkgs-review-checks extension. It is checked by a human on a best effort basis and does not build all packages (e.g. lumo, tensorflow or pytorch). Result of 13 packages built:
|
This is a semi-automatic executed nixpkgs-review with nixpkgs-review-checks extension. It is checked by a human on a best effort basis and does not build all packages (e.g. lumo, tensorflow or pytorch). Result of 15 packages built:
|
Motivation for this change
The main motivation was to fix this failing build.
I think these are the minimal changes to fix the build. It's a bit concerning that so many new packages and so much renaming had to be done to fix this.
@freezeboy: Shall we just remove the old
gradient-sdk
andpaperspace
packages instead of adding all of these new packages? No other package depends on them, so removal should be easy.Things done
sandbox
innix.conf
on non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
./result/bin/
)nix path-info -S
before and after)