Fix BP tree corruption due to numpy uints #563
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Ideally we would test this on numpy v2, but I can't seem to get empress running with numpy v2 on my system (get a cython error from iow). in any case this should address the problem
far as i can tell, both my system and gh actions are using flake8 7.1.0, so idk why only gh actions complained about the 80 char line length. whatever it's worth fixing i guess
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #562 (which was the cause of the problem discussed in #561).
Strangely enough, I can't seem to get Empress' Python code running within a conda environment that has numpy 2 installed -- I get a weird error from
bp/_bp.pyxsaying thatValueError: numpy.dtype size changed, may indicate binary incompatibility. Expected 96 from C header, got 88 from PyObject. But presumably it is possible to get numpy 2 and empress working together, because that was the set of circumstances that led to #561.Anyway, all this to say that this PR should fix #562 and make Empress compatible with numpy 2. I can't guarantee that, since I haven't been able to get iow + numpy 2 to play nicely together on my system ... but I imagine we'll eventually need to support numpy 2 anyway, so we may as well address this now.
Edit: the build is broken for other reasons (the main build is broken due to -- it looks like -- Q2 2020.6 not being installable any more, due to seaborn version jank?; and the standalone python 3.7 build is broken due to an old version of skbio). I guess fixing the build will be another PR ...