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

Updated vendored libraries #277

Closed
wants to merge 14 commits into from
Closed

Conversation

JoOkuma
Copy link
Contributor

@JoOkuma JoOkuma commented Jul 16, 2024

Hi @PerretB , I updated the libraries, this should close #276

It's passing on my environment with numpy==2.0.0.

If this gets merged, could we bump a version?

Note that I also removed an unused line in include/higra/image/graph_image.hpp to silence a warning.

@PerretB
Copy link
Member

PerretB commented Jul 17, 2024

Hi Jordão, thank you for the PR. There is a problem with catch2, they switched from a single file header only model in v2 to a multi-header statically compiled library in v3. So moving to v3 requires a lot of changes, if you are ready to make them you are of course welcome, but it is also possible to stick to v2 for now :)

No problem for increasing version number, once this is merged

@JoOkuma
Copy link
Contributor Author

JoOkuma commented Jul 17, 2024

Thanks for the heads up @PerretB , rolled back catch2

@JoOkuma
Copy link
Contributor Author

JoOkuma commented Jul 17, 2024

Just a heads up, I updated the CI of the three latest versions of Python

@PerretB
Copy link
Member

PerretB commented Jul 18, 2024

It seems that setuptools is being removed from default packages in 3.12 distributions. I think adding it there

CIBW_BEFORE_BUILD: "pip install cmake numpy==1.26.0"
(could be added to the other entries of the build matrix too) should fix the problem on Windows build (not sure why increasing Numpy version solved the issue on linux/macos builds, setuptools should probably also be explicitly added there)

@JoOkuma
Copy link
Contributor Author

JoOkuma commented Jul 18, 2024

Hey @PerretB, thanks for the feedback.
I'm not familiar with this CI, I added the setuptools install where I thought it was more appropriate, feel free to edit if you disagree with it.

@PerretB
Copy link
Member

PerretB commented Jul 19, 2024

ok indeed it's linked to this issue in xtensor xtensor-stack/xtensor#2736 , already reported as compiler bug in MSVC https://developercommunity.visualstudio.com/t/lambda-fails-to-implicitly-capture-constexpr-value/610504

@PerretB
Copy link
Member

PerretB commented Jul 19, 2024

I'm running out of ideas, I don't understand how xtensor tests are passing on MVSV :/

@PerretB
Copy link
Member

PerretB commented Jul 22, 2024

@JoOkuma I will try to replicate the problem locally and see if I can find a fix

@PerretB
Copy link
Member

PerretB commented Jul 22, 2024

@JoOkuma ok it's fixed. Do you mind creating a new clean PR with only required changes in small commits? Otherwise I'll do it during the week

@PerretB
Copy link
Member

PerretB commented Jul 23, 2024

replaced by #278

@PerretB PerretB closed this Jul 23, 2024
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

Successfully merging this pull request may close these issues.

Compatibility with numpy 2.0
2 participants