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.dateparser: switch to parameterized and thereby fix Hydra test #34643

Merged
merged 2 commits into from Feb 6, 2018

Conversation

dotlambda
Copy link
Member

@dotlambda dotlambda commented Feb 5, 2018

Motivation for this change

nose-parameterized is deprecated and yields a warning.
This fixes a failing Hydra test: https://hydra.nixos.org/build/68773666

The relevant upstream PR (scrapinghub/dateparser#381) can't be used for fetchpatch. The codebase seems to have changed too much in the meantime.

See also #34589.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option build-use-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 nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.

/cc @Ma27

@dotlambda dotlambda requested a review from FRidh as a code owner February 5, 2018 23:56
nose-parameterized is deprecated and yields a warning
@dotlambda dotlambda changed the title pythonPackages.dateparser: switch to parameterized and thereby fix Hydra builds pythonPackages.dateparser: switch to parameterized and thereby fix Hydra test Feb 6, 2018
@FRidh
Copy link
Member

FRidh commented Feb 6, 2018

Seems like a good solution to me. While you're at it, could you also remove Warning: after the warn
https://github.com/NixOS/nixpkgs/pull/34589/files#diff-2afbba83e1b14f9e0c304c769831a0e2R3842

@dotlambda
Copy link
Member Author

In the same commit?

@dotlambda
Copy link
Member Author

Removed Warning:

@Ma27
Copy link
Member

Ma27 commented Feb 6, 2018

@FRidh see my comment in the other PR, shouldn't we rather drop the warn expression for now and try to get rid of all usages or shall we make python builds pass if a warning is rendered?

@dotlambda
Copy link
Member Author

I grepped through python-packages.nix and development/python-modules/ and this was the only occurence of nose-parameterized.

@FRidh
Copy link
Member

FRidh commented Feb 6, 2018

Let's just get rid of this deprecated package as it simplifies things.

@Ma27
Copy link
Member

Ma27 commented Feb 6, 2018

I grepped through python-packages.nix and development/python-modules/ and this was the only occurence of nose-parameterized.

Thanks for your clarification, then you're right :)

Let's just get rid of this deprecated package as it simplifies things.

I wasn't sure at first if we should really drop this package, however I think that you might be right.
Currently preparing a PR for dateparser to get rid of the old dependency :-)

EDIT: @dotlambda was faster, thank you :-D

@dotlambda
Copy link
Member Author

I already made a PR: scrapinghub/dateparser#381

@dotlambda
Copy link
Member Author

I ran nix-build nixos/release-combined.nix -A nixpkgs.tarball again. No errors as expected.

@Ma27
Copy link
Member

Ma27 commented Feb 6, 2018

awesome! Thanks for caring about this <3

@dotlambda
Copy link
Member Author

@FRidh Please merge.

@FRidh FRidh merged commit ec26c65 into NixOS:master Feb 6, 2018
@dotlambda dotlambda deleted the dateparser branch February 7, 2018 08:07
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

4 participants