Skip to content

python3Packages.dedupe-pylbfgs: fix tests - #519894

Merged
pbsds merged 1 commit into
NixOS:masterfrom
samestep:dedupe-pylbfgs-fix-tests-numpy-2.4
May 19, 2026
Merged

python3Packages.dedupe-pylbfgs: fix tests#519894
pbsds merged 1 commit into
NixOS:masterfrom
samestep:dedupe-pylbfgs-fix-tests-numpy-2.4

Conversation

@samestep

@samestep samestep commented May 13, 2026

Copy link
Copy Markdown
Contributor

This package got broken by #488406 bumping NumPy from 2.3 to 2.4; see dedupeio/pylbfgs#52.

Disclosure: this patch was written by GPT-5.5 via Codex.

ZHF: #516381

Things done

  • Built on platform:
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • Tested, as applicable:
  • Ran nixpkgs-review on this PR. See nixpkgs-review usage.
  • Tested basic functionality of all binary files, usually in ./result/bin/.
  • Nixpkgs Release Notes
    • Package update: when the change is major or breaking.
  • NixOS Release Notes
    • Module addition: when adding a new NixOS module.
    • Module update: when the change is significant.
  • Fits CONTRIBUTING.md, pkgs/README.md, maintainers/README.md and other READMEs.

@samestep samestep added the 0.kind: ZHF Fixes Fixes during the Zero Hydra Failures (ZHF) campaign label May 13, 2026
@nixpkgs-ci
nixpkgs-ci Bot requested a review from daniel-fahey May 13, 2026 19:54
@nixpkgs-ci nixpkgs-ci Bot added 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 6.topic: python Python is a high-level, general-purpose programming language. labels May 13, 2026
@samestep

Copy link
Copy Markdown
Contributor Author

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 519894
Commit: c2a9b1d1895e1ce781a168c017a6949e7987e589


x86_64-linux

✅ 16 packages built:
  • python313Packages.dedupe
  • python313Packages.dedupe-pylbfgs
  • python313Packages.dedupe-pylbfgs.dist
  • python313Packages.dedupe.dist
  • python313Packages.highered
  • python313Packages.highered.dist
  • python313Packages.pyhacrf-datamade
  • python313Packages.pyhacrf-datamade.dist
  • python314Packages.dedupe
  • python314Packages.dedupe-pylbfgs
  • python314Packages.dedupe-pylbfgs.dist
  • python314Packages.dedupe.dist
  • python314Packages.highered
  • python314Packages.highered.dist
  • python314Packages.pyhacrf-datamade
  • python314Packages.pyhacrf-datamade.dist

@daniel-fahey daniel-fahey left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for getting to the bottom of this! Just one nit: I prefer to use fetchpatch2 instead of vendoring patch files (https://wiki.nixos.org/wiki/Nixpkgs/Patching_Nixpkgs)

{
lib,
buildPythonPackage,
fetchFromGitHub,

This comment was marked as outdated.

};

patches = [
./tests-numpy-2.4.patch # https://github.com/dedupeio/pylbfgs/pull/52

This comment was marked as outdated.

Comment on lines +1 to +25
diff --git a/tests/test_lbfgs.py b/tests/test_lbfgs.py
index 1af91ef..65e2da9 100644
--- a/tests/test_lbfgs.py
+++ b/tests/test_lbfgs.py
@@ -28,16 +28,16 @@ class TestOWLQN:

def test_owl_line_search_default(self):
def f(x, g, *args):
- g[0] = 2 * x
- return x ** 2
+ g[0] = 2 * x[0]
+ return x[0] ** 2

with pytest.warns(UserWarning, match="OWL-QN"):
xmin = fmin_lbfgs(f, 100., orthantwise_c=1)

def test_owl_line_search_warning_explicit(self):
def f(x, g, *args):
- g[0] = 2 * x
- return x ** 2
+ g[0] = 2 * x[0]
+ return x[0] ** 2

with pytest.warns(UserWarning, match="OWL-QN"):
xmin = fmin_lbfgs(f, 100., orthantwise_c=1, line_search='default')

This comment was marked as outdated.

@samestep

Copy link
Copy Markdown
Contributor Author

@daniel-fahey gotcha; for some reason I thought fetchpatch/fetchpatch2 were only preferred for patches that had already been merged upstream. Just to confirm, you're saying you prefer it even in this case where the upstream patch hasn't been merged yet?

@daniel-fahey

Copy link
Copy Markdown
Contributor

You might want to add e.g. Assisted-by: Codex/gpt-5.5 to your Nixpkgs and/or upstream commit too, which is the new convention.

@daniel-fahey

Copy link
Copy Markdown
Contributor

@daniel-fahey gotcha; for some reason I thought fetchpatch/fetchpatch2 were only preferred for patches that had already been merged upstream. Just to confirm, you're saying you prefer it even in this case where the upstream patch hasn't been merged yet?

Yeah, I'd also say it's especially useful since it's not been merged yet, I find it a lot easier to replace one url, get a new hash, than carefully fiddle around with patch files.

@samestep

Copy link
Copy Markdown
Contributor Author

OK makes sense! I'll swap it out once I'm back at my desk in a couple hours.

@daniel-fahey

daniel-fahey commented May 14, 2026

Copy link
Copy Markdown
Contributor

OK makes sense! I'll swap it out once I'm back at my desk in a couple hours.

Good, because after double checking the documentation I think you were right not to use fetchpatch2 😅.

nixpkgs/pkgs/README.md

Lines 637 to 638 in 8b02b33

When adding patches in this manner you should be reasonably sure that the used URL is stable.
Patches referencing open pull requests will change when the PR is updated and code forges (such as GitHub) usually garbage collect commits that are no longer reachable due to rebases/amends.

I'm "reasonably sure that the used URL is stable", but you've done it by the book, I withdraw my nit.

@nixpkgs-ci nixpkgs-ci Bot added 12.approvals: 1 This PR was reviewed and approved by one person. 12.approved-by: package-maintainer This PR was reviewed and approved by a maintainer listed in any of the changed packages. labels May 14, 2026
@samestep

Copy link
Copy Markdown
Contributor Author

Oh OK! I'll leave this PR as-is, then.

@SkohTV SkohTV left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nixpkgs-review result for #519894

Generated using nixpkgs-review-gha

Command: nixpkgs-review pr 519894
Commit: c2a9b1d1895e1ce781a168c017a6949e7987e589 (subsequent changes)
Merge: 5d7d954eec1071c63cfdf9877638fb7d14086f26

Logs: https://github.com/SkohTV/nixpkgs-review-gha/actions/runs/25919509522


x86_64-linux

✅ 16 packages built:
  • python313Packages.dedupe
  • python313Packages.dedupe-pylbfgs
  • python313Packages.dedupe-pylbfgs.dist
  • python313Packages.dedupe.dist
  • python313Packages.highered
  • python313Packages.highered.dist
  • python313Packages.pyhacrf-datamade
  • python313Packages.pyhacrf-datamade.dist
  • python314Packages.dedupe
  • python314Packages.dedupe-pylbfgs
  • python314Packages.dedupe-pylbfgs.dist
  • python314Packages.dedupe.dist
  • python314Packages.highered
  • python314Packages.highered.dist
  • python314Packages.pyhacrf-datamade
  • python314Packages.pyhacrf-datamade.dist

aarch64-linux

✅ 16 packages built:
  • python313Packages.dedupe
  • python313Packages.dedupe-pylbfgs
  • python313Packages.dedupe-pylbfgs.dist
  • python313Packages.dedupe.dist
  • python313Packages.highered
  • python313Packages.highered.dist
  • python313Packages.pyhacrf-datamade
  • python313Packages.pyhacrf-datamade.dist
  • python314Packages.dedupe
  • python314Packages.dedupe-pylbfgs
  • python314Packages.dedupe-pylbfgs.dist
  • python314Packages.dedupe.dist
  • python314Packages.highered
  • python314Packages.highered.dist
  • python314Packages.pyhacrf-datamade
  • python314Packages.pyhacrf-datamade.dist

x86_64-darwin (sandbox = relaxed)

✅ 16 packages built:
  • python313Packages.dedupe
  • python313Packages.dedupe-pylbfgs
  • python313Packages.dedupe-pylbfgs.dist
  • python313Packages.dedupe.dist
  • python313Packages.highered
  • python313Packages.highered.dist
  • python313Packages.pyhacrf-datamade
  • python313Packages.pyhacrf-datamade.dist
  • python314Packages.dedupe
  • python314Packages.dedupe-pylbfgs
  • python314Packages.dedupe-pylbfgs.dist
  • python314Packages.dedupe.dist
  • python314Packages.highered
  • python314Packages.highered.dist
  • python314Packages.pyhacrf-datamade
  • python314Packages.pyhacrf-datamade.dist

aarch64-darwin (sandbox = relaxed)

✅ 16 packages built:
  • python313Packages.dedupe
  • python313Packages.dedupe-pylbfgs
  • python313Packages.dedupe-pylbfgs.dist
  • python313Packages.dedupe.dist
  • python313Packages.highered
  • python313Packages.highered.dist
  • python313Packages.pyhacrf-datamade
  • python313Packages.pyhacrf-datamade.dist
  • python314Packages.dedupe
  • python314Packages.dedupe-pylbfgs
  • python314Packages.dedupe-pylbfgs.dist
  • python314Packages.dedupe.dist
  • python314Packages.highered
  • python314Packages.highered.dist
  • python314Packages.pyhacrf-datamade
  • python314Packages.pyhacrf-datamade.dist

@nixpkgs-ci nixpkgs-ci Bot added 12.approvals: 2 This PR was reviewed and approved by two persons. and removed 12.approvals: 1 This PR was reviewed and approved by one person. labels May 15, 2026
Assisted-by: Codex:gpt-5.5
@samestep
samestep force-pushed the dedupe-pylbfgs-fix-tests-numpy-2.4 branch from c2a9b1d to bf36349 Compare May 15, 2026 17:03
@pbsds
pbsds added this pull request to the merge queue May 19, 2026
Merged via the queue into NixOS:master with commit d807013 May 19, 2026
29 of 31 checks passed
@samestep
samestep deleted the dedupe-pylbfgs-fix-tests-numpy-2.4 branch May 19, 2026 14:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

0.kind: ZHF Fixes Fixes during the Zero Hydra Failures (ZHF) campaign 6.topic: python Python is a high-level, general-purpose programming language. 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 12.approvals: 2 This PR was reviewed and approved by two persons. 12.approved-by: package-maintainer This PR was reviewed and approved by a maintainer listed in any of the changed packages.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants