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

python3.pkgs.ml-dtypes: unpin build dependencies #250261

Closed
wants to merge 1 commit into from
Closed

python3.pkgs.ml-dtypes: unpin build dependencies #250261

wants to merge 1 commit into from

Conversation

tjni
Copy link
Contributor

@tjni tjni commented Aug 19, 2023

Description of changes

Also disable some failing tests on aarch64-darwin.

This will help this package build once we switch from pip to build. 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

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 23.11 Release Notes (or backporting 23.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Also disable some failing tests on aarch64-darwin.
@tjni
Copy link
Contributor Author

tjni commented Aug 19, 2023

@ofborg build python310Packages.ml-dtypes python311Packages.ml-dtypes

Copy link
Contributor

@GaetanLepage GaetanLepage left a 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
}:
Copy link
Contributor

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).

Comment on lines +32 to +37
postPatch = ''
substituteInPlace pyproject.toml \
--replace 'numpy~=' 'numpy>=' \
--replace 'pybind11~=' 'pybind11>=' \
--replace 'setuptools~=' 'setuptools>='
'';
Copy link
Member

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?

Copy link
Contributor Author

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.

Comment on lines +62 to +63
# These tests have expected RuntimeWarning: invalid value encountered in cast
# that have not been ignored yet upstream
Copy link
Member

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

Copy link
Contributor Author

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.

Copy link
Member

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

@tjni tjni marked this pull request as draft August 23, 2023 21:49
@wegank wegank added 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md 2.status: merge conflict This PR has merge conflicts with the target branch labels Mar 19, 2024
@tjni tjni closed this by deleting the head repository Aug 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.status: merge conflict This PR has merge conflicts with the target branch 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md 6.topic: python 10.rebuild-darwin: 1-10 10.rebuild-linux: 11-100
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants