-
-
Notifications
You must be signed in to change notification settings - Fork 14.8k
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
python3.pkgs.ml-dtypes: unpin build dependencies #250261
Conversation
Also disable some failing tests on aarch64-darwin.
@ofborg build python310Packages.ml-dtypes python311Packages.ml-dtypes |
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.
Thank you for this fix !
, pytestCheckHook | ||
, absl-py | ||
, setuptools | ||
, wheel | ||
}: |
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.
Please order the inputs by order of appearance (except for lib
to set first).
postPatch = '' | ||
substituteInPlace pyproject.toml \ | ||
--replace 'numpy~=' 'numpy>=' \ | ||
--replace 'pybind11~=' 'pybind11>=' \ | ||
--replace 'setuptools~=' 'setuptools>=' | ||
''; |
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.
why is this necessary/an improvement? in other words, if it ain't broke why fix it?
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 will break in staging
now since #248866 was merged.
# These tests have expected RuntimeWarning: invalid value encountered in cast | ||
# that have not been ignored yet upstream |
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.
what's the upstream issue tracking this? let's make sure to link to it
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 haven't found the time to research exactly if this is still an upstream issue or our issue. (jax-ml/ml_dtypes#11 in v0.2.0 looks like it's supposed to fix these, but it doesn't.) I will try to find time to do this.
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.
yeah let's either fix on our end or report upstream and link to the upstream issue in a code comment
Description of changes
Also disable some failing tests on aarch64-darwin.
This will help this package build once we switch from
pip
tobuild
. It looks like upstream chooses to pin these dependencies to their minor version, but it's a lot to keep it up to date in nixpkgs, so I chose to unpin it in a postPatch instead of fetching patches from upstream.Things done
sandbox = true
set innix.conf
? (See Nix manual)nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD"
. Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/
)