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

Use oldest-supported-numpy #208

Open
henryiii opened this issue May 19, 2022 · 6 comments
Open

Use oldest-supported-numpy #208

henryiii opened this issue May 19, 2022 · 6 comments

Comments

@henryiii
Copy link

I just noticed
https://github.com/dwavesystems/dwave-preprocessing/blob/be12b72b1e9c8ef450f83e93b844801df641e7bd/pyproject.toml#L6

This should almost always be oldest-supported-numpy if it's in a pyproject.toml file. https://github.com/scipy/oldest-supported-numpy - see https://scikit-hep.org/developer/packaging#special-additions-numpy or the note at https://cibuildwheel.readthedocs.io/en/stable/options/#before-build for example.

@arcondello
Copy link
Member

Hi @henryiii , I do think that's a cool package, and we may use it in the future. However, we do currently test our supported NumPy range to ensure that the build version is compatible with all of our supported versions (not every NumPy minor version increment introduces a binary incompatibility).

Have you run into any bugs or specific issues? Otherwise I will mark this as a feature request rather than a bug. Thanks!

@henryiii
Copy link
Author

henryiii commented May 19, 2022

not every NumPy minor version increment introduces a binary incompatibility

I think you mean patch, not minor. The NumPy team test oldest-supported-numpy against each patch release. Binary release automatically are always considered not backward compatible by a check, regardless of if they are. Forward compatibility is guaranteed, backward compatibly for a minor version is guarantied to be non-compatible, and could even be for a patch version.

What I'd recommend, if you use any newer header features that need a recent NumPy, is setting the minimum version manually if you want for the cases in https://github.com/scipy/oldest-supported-numpy/blob/66fc65cfa7bfbd621f0b9d4758ab800afa54c617/setup.cfg#L21-L67, then allow all newer Pythons and rarer architectures use oldest-supported-numpy since that is maintained by the NumPy team.

Have you run into any bugs or specific issues? Otherwise I will mark this as a feature request rather than a bug.

Nope, just noticed this checking unrelated breakages in conda-forge (related to pip 22.1.0).

@henryiii
Copy link
Author

henryiii commented May 19, 2022

Also, on the numpy<2 mention there: https://iscinumpy.dev/post/bound-version-constraints/. Also NumPy is not semantically versioned. They generally try to promise 3 update support (no warnings in 1.x should work (maybe with warnings) till 1.x+3). Though I'd still argue that's not correct to preemptively cap on.

I'll go put my nose back in my own business now! Cheers. :)

@arcondello
Copy link
Member

arcondello commented May 19, 2022

I think you mean patch, not minor.

I did mean minor. Though I agree/know that in general one needs to treat minor changes as breaking, my point was that we specifically tested the 1.20-1.21 increment to ensure that it was not incompatible (or at least not for the parts we were using). Though I am very aware that it's possible we missed something.

What I'd recommend, if you use any newer header features that need a recent NumPy, is setting the minimum version manually if you want for the cases in https://github.com/scipy/oldest-supported-numpy/blob/66fc65cfa7bfbd621f0b9d4758ab800afa54c617/setup.cfg#L21-L67, then allow all newer Pythons and rarer architectures use oldest-supported-numpy since that is maintained by the NumPy team.

Totally agree. For the majority of our packages it would be good to just abstract the requirement away to the NumPy maintainers. We'll likely start migrating to this approach in our next version update cycle.

Also, on the numpy<2 mention there: https://iscinumpy.dev/post/bound-version-constraints/. Also NumPy is not semantically versioned. They generally try to promise 3 update support (no warnings in 1.x should work (maybe with warnings) till 1.x+3). Though I'd still argue that's not correct to preemptively cap on.

Also agree, we're not very consistent with this accross Ocean (e.g. https://github.com/dwavesystems/dimod/blob/7826a91192d547b9618da28a48d9fd067a3c0085/setup.py#L82 does include a cap, but we don't here)

I'll go put my nose back in my own business now! Cheers. :)

We appreciate folks reaching out! Truly! This is the best part of open source. And I now know of a cool utility package that I didn't before.

@BastianZim
Copy link

As the person, that (probably) initiated this discussion on conda-forge :) Would be awesome to have a wider range of NumPy available as I currently need to do some manual adjusting to get every package of the SDK to be compatible with each other. (This applies to the whole of the ocean SDK though, not only here).

@arcondello
Copy link
Member

I am going to transfer this issue over to our sdk, so that we can track changes across all of Ocean

@arcondello arcondello transferred this issue from dwavesystems/dwave-preprocessing May 30, 2022
arcondello added a commit to arcondello/dimod that referenced this issue May 30, 2022
Although we need some newer features of NumPy at runtime, we
can safely build against the older headers. I was not able
to detect any performance regressions from doing so either.

See dwavesystems/dwave-ocean-sdk#208
arcondello added a commit to arcondello/dimod that referenced this issue May 30, 2022
Although we need some newer features of NumPy at runtime, we
can safely build against the older headers. I was not able
to detect any performance regressions from doing so either.

See dwavesystems/dwave-ocean-sdk#208
arcondello added a commit to arcondello/dwave-preprocessing that referenced this issue May 31, 2022
arcondello added a commit to arcondello/dwave-preprocessing that referenced this issue May 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants