-
-
Notifications
You must be signed in to change notification settings - Fork 14.3k
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
python312Packages.wandb: 0.16.0 -> 0.18.5 #350098
Conversation
0be73c9
to
bf25bc4
Compare
nice, wandb update :) |
I am still battling with a few tests but the package seems to be working nicely overall. |
I got it to build on linux, but the rust binary fails to compile on darwin:
|
89f1b7c
to
b137136
Compare
I'm seeing the aarch64-darwin build fail for a different reason:
@GaetanLepage is there something different about your setup? FWIW I'm happy to merge without darwin support since building on linux is an improvement over the current status quo. |
I am about to push the following which I think is better: index 3de94a2a99f1..676c46295095 100644
--- a/pkgs/development/python-modules/wandb/default.nix
+++ b/pkgs/development/python-modules/wandb/default.nix
@@ -162,22 +162,34 @@ buildPythonPackage rec {
})
];
+ env = {
+ # https://github.com/wandb/wandb/blob/v0.18.5/hatch_build.py#L37-L47
+
+ # Prevent the install script to try building and embedding the `gpu_stats` binary in the wheel.
+ # It will be patched within the `wandb-core` source code.
+ WANDB_BUILD_SKIP_GPU_STATS = true;
+
+ # Prevent the install script to try building and embedding the `wandb-core` binary in the wheel.
+ # It will be patched within the `wandb` source code.
+ WANDB_BUILD_UNIVERSAL = true;
+ };
+
# We manually compile those two dependencies and inject their path to bypass their build scripts
postPatch = ''
- substituteInPlace hatch_build.py \
- --replace-fail \
- 'output = pathlib.Path("wandb", "bin", "wandb-core")' \
- 'return ["${lib.getExe wandb-core}"]' \
- --replace-fail \
- 'output = pathlib.Path("wandb", "bin", "gpu_stats")' \
- 'return ["${lib.getExe gpu-stats}"]'
-
substituteInPlace wandb/util.py \
--replace-fail \
'bin_path = pathlib.Path(__file__).parent / "bin" / "wandb-core"' \
'bin_path = pathlib.Path("${lib.getExe wandb-core}")'
''; Indeed, currently I am still "embedding" the |
Also, I removed the |
8325f3e
to
aa17670
Compare
|
Ok @samuela, this is good to go according to me. Please, have a look at it to spot eventual mistakes.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is great -- thanks so much @GaetanLepage! I'm a little confused how __darwinAllowLocalNetworking
is intended to be used, but o/w I think this looks good.
pytestCheckHook | ||
]; | ||
|
||
__darwinAllowLocalNetworking = true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like this is deprecated/renamed:
nixpkgs/pkgs/stdenv/generic/make-derivation.nix
Lines 130 to 137 in 7783a48
removedOrReplacedAttrNames = [ | |
"checkInputs" "installCheckInputs" | |
"nativeCheckInputs" "nativeInstallCheckInputs" | |
"__contentAddressed" | |
"__darwinAllowLocalNetworking" | |
"__impureHostDeps" "__propagatedImpureHostDeps" | |
"sandboxProfile" "propagatedSandboxProfile" | |
]; |
but I'm not able to find any docs pointing to why or what to use instead.
Perhaps @NixOS/darwin-maintainers might be able to help us?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apparently, it will just be removed from the derivation arguments and we can still use it without any problem.
# Expects python binary to be named `python3` but nix provides `python3.12` | ||
# AssertionError: assert ['python3.12', 'main.py'] == ['python3', 'main.py'] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❯ ns -p python312
[nix-shell:~]$ which python3
/nix/store/ybnf7k6i9p244bbhsbxizqk65z58cwyr-python3-3.12.6/bin/python3
perhaps I'm missing something here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know, this is weird. But I confirm that these tests fails:
_______________________ test_get_entrypoint[wandb_core] ________________________
[gw0] linux -- Python 3.12.6 /nix/store/wfbjq35kxs6x83c3ncpfxdyl5gbhdx4h-python3-3.12.6/bin/python3.12
@pytest.mark.skipif(
platform.system() == "Windows",
reason="python exec name is different on windows",
)
def test_get_entrypoint():
dir = tempfile.TemporaryDirectory().name
job_source = "artifact"
builder = _configure_job_builder_for_partial(dir, job_source)
metadata = {"python": "3.9.11", "codePathLocal": "main.py", "_partial": "v0"}
program_relpath = builder._get_program_relpath(job_source, metadata)
entrypoint = builder._get_entrypoint(program_relpath, metadata)
> assert entrypoint == ["python3", "main.py"]
E AssertionError: assert ['python3.12', 'main.py'] == ['python3', 'main.py']
E
E At index 0 diff: 'python3.12' != 'python3'
E
E Full diff:
E [
E - 'python3',
E + 'python3.12',
E ? +++
E 'main.py',
E ]
/build/source/tests/unit_tests/test_launch/test_create_job.py:126: AssertionError
@samuela do you have any further concern ? |
I took the decision to merge it so we can move on, let's iterate on this later if @samuela has some other concerns. |
Things done
Changelog: https://github.com/wandb/wandb/raw/v0.18.5/CHANGELOG.md
cc @samuela
fixes #349691
nix.conf
? (See Nix manual)sandbox = relaxed
sandbox = true
nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD"
. Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/
)Add a 👍 reaction to pull requests you find important.