support `numpy>=2` - Githubissues

bayesian-optimization / BayesianOptimization

A Python implementation of global optimization with gaussian processes.

https://bayesian-optimization.github.io/BayesianOptimization/index.html

MIT License

7.95k stars 1.55k forks source link

support `numpy>=2` #490

Closed phi-friday closed 4 months ago

phi-friday commented 4 months ago

Is your feature request related to a problem? Please describe. not yet support numpy>=2

Describe the solution you'd like in pyproject.toml, numpy = "^1.9.0" -> numpy = ">=1.25" (Since version 1.25, numpy does not support python<=3.8.)

References or alternative approaches If this feature was described in literature, please add references here. Additionally, feel free to add descriptions of any alternative solutions or features you've considered.

Additional context Add any other context or screenshots about the feature request here.

Are you able and willing to implement this feature yourself and open a pull request?

[X] Yes, I can provide this feature.

till-m commented 4 months ago

Hi @phi-friday,

thanks for creating this issue. I'm a bit confused, the title of the issue refers to numpy 2.0.0 but the content is about supporting numpy <1.9.0. Can you clear this up for me maybe?

Regarding the former, I haven't had the time to check what, if any, adjustments would need to be made to the code.
Regarding the latter, I think we intentionally dropped python 3.8 support. @bwheelz36 do you remember why?

bwheelz36 commented 4 months ago

I think numpy or some other dependency was no longer supporting it. But I don't remember very clearly tbh.

changes to be made: maybe none. So far the only issues I've had switching to numpy 2 has been in typing (they dropped some deprecated types). someone could check quickly if all our tests pass with numpy 2
BUT. I'm not sure it's a good idea to be supporting both numpy 1.x and 2.x as is proposed by @phi-friday. There presumably will be breaking changes between these versions even if they don't currently break this code. Eventually we will have to move to 2, but at that point we should drop support for 1, which will be breaking for most users.

It's definitely possible right now for users to use this package with numpy 2 with a bit of fiddling. I'd probably prefer to hold off a little longer until upgrading to 2. But I'm open to counter arguments.

bwheelz36 commented 4 months ago

Oh, just saw that indeed all tests are passing. Thanks @phi-friday!

till-m commented 4 months ago

Found it!

I think it's probably not necessary to support 3.8. IF we want to enable numpy 2.0.0, I would opt for numpy = ">1.9.0".

I'd probably prefer to hold off a little longer until upgrading to 2.

I agree.

phi-friday commented 4 months ago

Hi @phi-friday,

thanks for creating this issue. I'm a bit confused, the title of the issue refers to numpy 2.0.0 but the content is about supporting numpy <1.9.0. Can you clear this up for me maybe?

Regarding the former, I haven't had the time to check what, if any, adjustments would need to be made to the code.

Regarding the latter, I think we intentionally dropped python 3.8 support. @bwheelz36 do you remember why?

According to https://python-poetry.org/docs/dependency-specification/, ^1.9 does not support >=2.0.0.

Found it!

I think it's probably not necessary to support 3.8. IF we want to enable numpy 2.0.0, I would opt for numpy = ">1.9.0".

I'd probably prefer to hold off a little longer until upgrading to 2.

I agree.

If you're not going to support python<=3.8, why not limit it to >=1.25?

phi-friday commented 4 months ago

I think numpy or some other dependency was no longer supporting it. But I don't remember very clearly tbh.

changes to be made: maybe none. So far the only issues I've had switching to numpy 2 has been in typing (they dropped some deprecated types). someone could check quickly if all our tests pass with numpy 2

BUT. I'm not sure it's a good idea to be supporting both numpy 1.x and 2.x as is proposed by @phi-friday. There presumably will be breaking changes between these versions even if they don't currently break this code. Eventually we will have to move to 2, but at that point we should drop support for 1, which will be breaking for most users.

It's definitely possible right now for users to use this package with numpy 2 with a bit of fiddling. I'd probably prefer to hold off a little longer until upgrading to 2. But I'm open to counter arguments.

numpy>=2 has been in the works for a very long time. If you haven't encountered a DeprecationWarning, you should be fine in most cases.

However, just in case, if you plan to support both v1 and v2, it seems like a good idea to add an action for each.

brendan-whelan-seetreat commented 4 months ago

agreed @phi-friday , I think if both versions are being explicitly tested via CI we could support both.

If we just change to numpy = ">=1.25" as proposed, I think we'd have to do some manual fiddling to make sure numpy <2.0.0 is tested, since I think poetry would install 2.0 for every python version > 3.8.

So, we could:

run the tests as normal
then pip uninstall numpy
pip install "numpy<2.0.0"
then run the tests again

note that I haven't checked that would work but I think it would

till-m commented 4 months ago

You can handle different version with a matrix. There's actually an example for exactly this case in snok/install-poetry's docs.

phi-friday commented 4 months ago

You can handle different version with a matrix. There's actually an example for exactly this case in snok/install-poetry's docs.

https://github.com/bayesian-optimization/BayesianOptimization/pull/491#issuecomment-2213772825

I added new action, and it passed all tests.