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.

Sign up for GitHub

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

Jump to bottom

pandas: add 'which' dependency #26576

Closed
wants to merge 1 commit into from
Closed

pandas: add 'which' dependency #26576

wants to merge 1 commit into from
+2 −0

Conversation

michaelpj
Copy link
Contributor

pandas has an undeclared dependency on which (it uses it to test for the presence of executables). You wouldn't notice it unless you were running in nix-shell --pure, which I was to troubleshoot other things.

Things done
  • Tested using sandboxing
    (nix.useSandbox on NixOS,
    or option build-use-sandbox in nix.conf
    on non-NixOS)
  • Built on platform(s)
    • [ x ] NixOS
    • macOS
    • Linux
  • 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.

Sorry, something went wrong.

@michaelpj
pandas: add 'which' dependency

Verified

This commit was signed with the committer’s verified signature.
edolstra Eelco Dolstra
GPG key ID: 8170B4726D7198DE
Verified
Learn about vigilant mode
cb97d68
@rasendubi
Copy link
Member

So this is runtime dependency, right? (Not build-time one)

@michaelpj
Copy link
Contributor Author

Yes, hence why I've put it in propagatedBuildInputs. I think that's the right thing to do? It seems to work at least.

@Mic92
Copy link
Member

Mic92 commented Jun 14, 2017 •
edited
Loading

Because you are in a nix-shell, which pulls propagatedBuildInputs. A better fix is to replace which in the source with the full path.

@michaelpj
Copy link
Contributor Author

Ah okay, that makes sense. I'll look into patching the source.

@rasendubi
Copy link
Member

Another option is to patch $PATH in the wrapper.

@michaelpj
Copy link
Contributor Author

There isn't a wrapper in this case because it's usually used as a library.

@FRidh
Copy link
Member

FRidh commented Jun 15, 2017

When does it need which during runtime? No issues showed up when building it in a sandbox and running the tests in it. If its straightforward to patch, then by all means, do so. Note that we don't always add every dependency; just what is considered needed for typical usage.

@FRidh FRidh self-assigned this Jun 15, 2017
@FRidh
Copy link
Member

FRidh commented Jul 17, 2017

Closing because of inactivity and because I couldn't find any occurrence of which in pandas.

@FRidh FRidh closed this Jul 17, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@FRidh FRidh

Labels
6.topic: python
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@michaelpj @rasendubi @Mic92 @FRidh