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

Feature: migrate to numpy v2.0.0 #596

Merged
merged 12 commits into from
Aug 9, 2024
Merged

Feature: migrate to numpy v2.0.0 #596

merged 12 commits into from
Aug 9, 2024

Conversation

mrava87
Copy link
Collaborator

@mrava87 mrava87 commented Jul 26, 2024

This PR aims to allow users to use pylops with numpy>=2.0.0.

Currently we need to make the following work-arounds:

  • disable tests for spgl1 until new release of spgl1 library
  • disable tests for DTCWT until new release of dtcwt library
  • disable tests for TorchOperator in darwin os (currently torch does give this error RuntimeError: Numpy is not available)

@mrava87 mrava87 marked this pull request as draft July 26, 2024 19:54
@mrava87 mrava87 marked this pull request as ready for review July 27, 2024 10:27
@mrava87 mrava87 requested a review from cako July 27, 2024 10:27
@mrava87
Copy link
Collaborator Author

mrava87 commented Jul 27, 2024

@cako can you please quickly review this one, so we can make a new release that works with numpy v2

@cako cako added enhancement New feature or request Core Pulls for core PyLops library labels Aug 5, 2024
Copy link
Collaborator

@cako cako left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving, LGTM. I added a couple of changes to the testing logic to reduce the diff, and I also added a note in the documentation about dtcwt and spgl1 not working with numpy2

@cako
Copy link
Collaborator

cako commented Aug 5, 2024

@mrava87 Apparently dtcwt has been archived. The new version lives in: https://github.com/xir4n/dtcwt

We might want to support that fork instead.

EDIT: Just saw that the fork is the one being used in PyPI. Nevermind!

@mrava87
Copy link
Collaborator Author

mrava87 commented Aug 9, 2024

@mrava87 Apparently dtcwt has been archived. The new version lives in: https://github.com/xir4n/dtcwt

We might want to support that fork instead.

EDIT: Just saw that the fork is the one being used in PyPI. Nevermind!

I saw this... it 'happened' as soon as I opened an Issue about numpy2 (rjw57/dtcwt#149).... now, I cannot reply anymore to this Issue thread and the fork does not have Issues (since it is a fork), so we seem to have lost all ways to communicate. I suggest that we keep this for now, it would not affect people using pylops, and consider ditching dtcwt all together in a few months if the library does not update to support numpy2. What do you think?

@mrava87 mrava87 merged commit 32bad36 into PyLops:dev Aug 9, 2024
13 checks passed
@mrava87 mrava87 deleted the patch_numpyv2 branch August 9, 2024 12:44
@cako
Copy link
Collaborator

cako commented Aug 19, 2024

I don't mind keeping it. If it becomes a pain to maintain, we can remove it in the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Core Pulls for core PyLops library enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants