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

Numpy v2 has a range of API changes. #889

Open
iandobbie opened this issue Jan 29, 2024 · 11 comments
Open

Numpy v2 has a range of API changes. #889

iandobbie opened this issue Jan 29, 2024 · 11 comments

Comments

@iandobbie
Copy link
Collaborator

Numpy is just about to upgrade to V2. There are a large number of API changes and I have no idea if they will affect cockoit or not but we probably ought to check. Migration guide...

https://numpy.org/devdocs/numpy_2_0_migration_guide.html

I will see if I see any obvious issues but we probably have to try it.

Ian

@iandobbie
Copy link
Collaborator Author

Julio suggests that we pin numpy to version 1 for a few months to allow any libraries we use that depend on numpy to be updated.

@carandraug
Copy link
Collaborator

I don't understand how we pinning to numpy v1 helps the other libraries. If we don't pin, then we're declaring we work with both v1 and v2, leaving the choice of what version to be installed up to pip (and pip will install an older version if it detects that the other libraries need that older version).

@juliomateoslangerak
Copy link
Contributor

The problem is that some key libraries (don't remember which ones) are not pinning down numpy 1 and they have not been tested with numpy 2. Not pinning down microscope to numpy 1 might lead to installation of numpy 2 and causing problems through those libraries. We should pin v1 until they fix their issues. I can check which libraries are causing the issue.

I think in general its a good practice to pin down the major versions (breaking changes) to avoid trouble.

@carandraug
Copy link
Collaborator

I just pushed 6f441e2 and 5326c4b which fix numpy 2 compatibility issues (there may be others though). Can someone test in actual microscope?

The problem is that some key libraries (don't remember which ones) are not pinning down numpy 1 and they have not been tested with numpy 2. Not pinning down microscope to numpy 1 might lead to installation of numpy 2 and causing problems through those libraries. We should pin v1 until they fix their issues. I can check which libraries are causing the issue.

I really don't think we should be pinning a requirement on numpy 1 for the sake of other libraries. Instead lets request the other libraries to pin it themselves.

I think in general its a good practice to pin down the major versions (breaking changes) to avoid trouble.

Pinning a requirement on an older version prevents the user from installing a newer, possible faster or less buggy version. It also prevents the user from installing other packages that may require the newer version.

Just because a newer version introduces breaking changes, it doesn't mean it will break things for us. And if we're good at keeping dependencies up to date then we can make it work with both. For example, all the fixes for numpy 2 still work with numpy 1.

@iandobbie
Copy link
Collaborator Author

I will try these updates on my test system with has a picamera, ludl stage and then simulated light sources but a red pitaya executor for digital and analog control. Might take a few days as I want to test with numpy 1 as it is then upgrade at least the windows pc and the py to numpy 2 and check it all still works. Not sure how easy it will be to upgrade the red pitaya numpy version.

@iandobbie
Copy link
Collaborator Author

iandobbie commented Jul 11, 2024

So I started to work on this. Installed a venv on windows with python 3.10 and installed numpy2 and other dependencies. Starting the code generated the attached errors.
numpy2-errors.txt

So there appear to be a bunch of GL errors triggered by freetype, additionally the final error appears to be caused by:

ERROR:cockpit.gui.loggingWindow:AttributeError: module 'numpy' has no attribute 'sometrue'

So this appears to be a real numpy2 issue.

@iandobbie
Copy link
Collaborator Author

Ok, after some reading it turns out that numpy.sometrue should be replaced with numpy.any but this is in wxPython so an upstream issue.

@carandraug
Copy link
Collaborator

This is triggered by cockpit.util.intensity because that window uses floatcanvas. We could replace floatcanvas (doesn't look like we use much of i, would be better if we used wxWidgets stuff only - floatcanvas is wxPython only). Would be nice if it was possible to select which windows to start.

@iandobbie
Copy link
Collaborator Author

Turns out most of this is a red herring. I was using remote desktop to my PC from my mac and totally forgot that this breaks the openGL stuff. There is the issue with the floatcanvas but this appears to be ignorable, or fixable by just replacing the sometrue with any. I have opened an issue with wxpython wxWidgets/Phoenix#2574

However, there is a different issue, which is related the David recent changes to MRC.py I will open a separate issue with this - #896

@iandobbie
Copy link
Collaborator Author

The floatpanel issue seems to be fixed by just changing the wxpython function to use numpy.any and there are no other references to this in wxpython as far as I can see. I will try to create a minimal test for the wxpython bug report.

@carandraug
Copy link
Collaborator

I have sent wxPython a a pull request that fixes this issue (see wxWidgets/Phoenix#2578). In the mean time, we can workaround this in cockpit by monkey patching it ourselves like this:

diff --git a/cockpit/util/intensity.py b/cockpit/util/intensity.py
index 2343cd1a..62dd9b06 100644
--- a/cockpit/util/intensity.py
+++ b/cockpit/util/intensity.py
@@ -37,6 +37,13 @@ from wx.lib.floatcanvas import FloatCanvas
 import wx.lib.plot as plot
 
 
+## HACK: wxPython uses np.sometrue which was deprecated in numpy 1.25
+## and removed in numpy 2.0.  np.any is a drop in replacement so it
+## should be fine to monkey patch it.  We need this while wx does not
+## fix it on their side (see #889).
+FloatCanvas.N.sometrue = np.any
+
+
 ICON_SIZE = (16,16)
 BITMAP_SIZE = (512,512)
 

But I think we're better off dropping the use FloatCanvas (and in the long run, most if not all that is in wx.lib)

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