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.imgaug: 0.3.0-patch #67494

Merged
merged 1 commit into from Jan 7, 2020
Merged

Conversation

@Rakesh4G
Copy link
Contributor

@Rakesh4G Rakesh4G commented Aug 26, 2019

Added patch to bypass opencv-python dependency from imgaug

@CMCDragonkai

Motivation for this change
Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • 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 @CMCDragonkai @FRidh

@Rakesh4G Rakesh4G changed the title imgaug: 0.2.9-patch pythonPackages.imgaug: 0.2.9-patch Aug 26, 2019
@ofborg ofborg bot added the 6.topic: python label Aug 26, 2019
@Rakesh4G
Copy link
Contributor Author

@Rakesh4G Rakesh4G commented Aug 26, 2019

package has been updated as per inputs from

#61936

aleju/imgaug#379 (comment)

@CMCDragonkai
Copy link
Member

@CMCDragonkai CMCDragonkai commented Aug 27, 2019

Note that in the master branch of imgaug, the author has changed to opencv-python-headless from opencv-python. See: https://github.com/aleju/imgaug/blob/master/setup.py#L21.

This means for this release 0.2.9, it should work, but the next release may require a change to the substitute patch.

Copy link
Member

@CMCDragonkai CMCDragonkai left a comment

Approved, looks good. Mark it ready for review.

@Rakesh4G Rakesh4G marked this pull request as ready for review Aug 27, 2019
@Rakesh4G Rakesh4G requested a review from FRidh as a code owner Aug 27, 2019
@mmahut
Copy link
Member

@mmahut mmahut commented Aug 27, 2019

@GrahamcOfBorg build python37Packages.imgaug

@mmahut
Copy link
Member

@mmahut mmahut commented Aug 27, 2019

Looks like resulting derivation is empty?

[nix-shell:~/.cache/nix-review/pr-67494/results/python37Packages.imgaug]# tree
.
└── nix-support
    └── propagated-build-inputs

1 directory, 1 file
@CMCDragonkai
Copy link
Member

@CMCDragonkai CMCDragonkai commented Sep 25, 2019

This is blocked on aleju/imgaug#414

@tbenst tbenst mentioned this pull request Nov 7, 2019
3 of 10 tasks complete
@Rakesh4G Rakesh4G requested a review from jonringer as a code owner Nov 13, 2019
@Rakesh4G Rakesh4G changed the title pythonPackages.imgaug: 0.2.9-patch pythonPackages.imgaug: 0.3.0-patch Nov 13, 2019
@jonringer
Copy link
Contributor

@jonringer jonringer commented Nov 13, 2019

do you mind rebasing on top of master? apparently there's some merge conflicts:

git pull -r origin master
git push <fork> <branch> --force
@Rakesh4G
Copy link
Contributor Author

@Rakesh4G Rakesh4G commented Nov 13, 2019

do you mind rebasing on top of master? apparently there's some merge conflicts:

git pull -r origin master
git push <fork> <branch> --force

Sure, I will.

@tbenst
Copy link
Contributor

@tbenst tbenst commented Nov 25, 2019

Given that imgaug is functioning on waiting on upstream for a fix, perhaps we should merge & fix broken package?

@Rakesh4G Rakesh4G force-pushed the formbay:imgaug-0.2.9-patch branch from e407d67 to 56740bc Dec 1, 2019
@Rakesh4G
Copy link
Contributor Author

@Rakesh4G Rakesh4G commented Dec 3, 2019

The tests hang issue aleju/imgaug#414 with imgaug-0.3.0 has been fixed on master. I have taken the latest changes and current expression is ready to be merged.
@tbenst @CMCDragonkai @jonringer

@CMCDragonkai
Copy link
Member

@CMCDragonkai CMCDragonkai commented Dec 11, 2019

When attempting to install the latest changes using nix-env -f ./default.nix -iA 'pkgs.python3Packages.imgaug, the tests for hang here:

test/augmenters/test_meta.py::TestAugmenter_augment_batches::test_augment_batches_list_of_list_of_invalid_datatype PASSED

The build just stops there, the pytest command is still running but with 0% CPU usage.

@Rakesh4G
Copy link
Contributor Author

@Rakesh4G Rakesh4G commented Dec 11, 2019

nix-env -f ./default.nix -iA 'pkgs.python3Packages.imgaug

@CMCDragonkai
yes, you are correct. I am also facing this hang issue with above command you mentioned.
looks like i had missed something. I will re-inspect and see what is going wrong.

@Rakesh4G
Copy link
Contributor Author

@Rakesh4G Rakesh4G commented Dec 11, 2019

I have updated the error details and other findings on aleju/imgaug#414

@Rakesh4G Rakesh4G force-pushed the formbay:imgaug-0.2.9-patch branch 2 times, most recently from 3470632 to 5b52dd5 Dec 12, 2019
@Rakesh4G
Copy link
Contributor Author

@Rakesh4G Rakesh4G commented Dec 19, 2019

This PR is ready for merge. All the tests are now passing.
@tbenst @jonringer @CMCDragonkai

Copy link
Contributor

@jonringer jonringer left a comment

diff LGTM
commit LGTM
tests pass 👍

[3 built, 0.0 MiB DL]
https://github.com/NixOS/nixpkgs/pull/67494
1 package were built:
python37Packages.imgaug
@jonringer
Copy link
Contributor

@jonringer jonringer commented Dec 19, 2019

@GrahamcOfBorg build python37Packages.imgaug

@jonringer jonringer mentioned this pull request Dec 19, 2019
0 of 10 tasks complete
@jonringer jonringer self-requested a review Dec 19, 2019
@CMCDragonkai
Copy link
Member

@CMCDragonkai CMCDragonkai commented Dec 20, 2019

@CMCDragonkai
Copy link
Member

@CMCDragonkai CMCDragonkai commented Dec 20, 2019

We don't have Mac platforms to test this. So we could forward on the details to imgaug maintainer. While also just only enabling this for Linux?

@CMCDragonkai
Copy link
Member

@CMCDragonkai CMCDragonkai commented Dec 20, 2019

Another idea is a patch in Nix package level that disables those specific tests.

@jonringer
Copy link
Contributor

@jonringer jonringer commented Dec 20, 2019

personally, I would either do doCheck = stdenv.isLinux or meta.platforms = platforms.linux. The meta platform is probably more appropriate if we don't have some other form of testing.

@Rakesh4G Rakesh4G force-pushed the formbay:imgaug-0.2.9-patch branch from 5b52dd5 to a9336e4 Dec 20, 2019
@Rakesh4G
Copy link
Contributor Author

@Rakesh4G Rakesh4G commented Dec 20, 2019

I have updated the requested changes. Thanks.
@FRidh @jonringer @CMCDragonkai

@jonringer
Copy link
Contributor

@jonringer jonringer commented Dec 20, 2019

@GrahamcOfBorg build python37Packages.imgaug

@tbenst
Copy link
Contributor

@tbenst tbenst commented Jan 7, 2020

I cherry-picked this patch and has been working for me! would love to see merged

@Rakesh4G Rakesh4G requested a review from FRidh Jan 7, 2020
@jonringer jonringer merged commit f412725 into NixOS:master Jan 7, 2020
16 checks passed
16 checks passed
python37Packages.imgaug on x86_64-darwin No attempt
Details
Evaluation Performance Report Evaluator Performance Report
Details
grahamcofborg-eval ^.^!
Details
grahamcofborg-eval-check-maintainers matching changed paths to changed attrs...
Details
grahamcofborg-eval-check-meta config.nix: checkMeta = true
Details
grahamcofborg-eval-darwin nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A darwin-tested
Details
grahamcofborg-eval-nixos nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./nixos/release-combined.nix -A tested
Details
grahamcofborg-eval-nixos-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./nixos/release.nix -A manual
Details
grahamcofborg-eval-nixos-options nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./nixos/release.nix -A options
Details
grahamcofborg-eval-nixpkgs-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A manual
Details
grahamcofborg-eval-nixpkgs-tarball nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A tarball
Details
grahamcofborg-eval-nixpkgs-unstable-jobset nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A unstable
Details
grahamcofborg-eval-package-list nix-env -qa --json --file .
Details
grahamcofborg-eval-package-list-no-aliases nix-env -qa --json --file . --arg config { allowAliases = false; }
Details
python37Packages.imgaug on aarch64-linux Success
Details
python37Packages.imgaug on x86_64-linux Success
Details
@Rakesh4G Rakesh4G deleted the formbay:imgaug-0.2.9-patch branch Jan 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

7 participants
You can’t perform that action at this time.