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

[MRG] Replace np.in1d() with np.isin() #799

Merged
merged 5 commits into from
Jun 20, 2024

Conversation

ntolley
Copy link
Contributor

@ntolley ntolley commented Jun 17, 2024

Problem raised in #794 that numpy version 2.0.0 has deprecated np.in1d()

@dylansdaniels
Copy link
Collaborator

@ntolley i can squash and merge per our discussion yesterday if we're ready to go. seems like a straightforward substitution

@rythorpe
Copy link
Contributor

Looks like there might be some flake8 errors, though it looks like don't have anything to do with this PR. Hmmm...

@jasmainak
Copy link
Collaborator

should you also bump up the minimum numpy version?

@ntolley
Copy link
Contributor Author

ntolley commented Jun 18, 2024

@rythorpe fixed the flake8 errors!

Also @jasmainak no need, np.isin() is already a part of numpy v1.14
https://numpy.org/doc/1.14/reference/generated/numpy.isin.html#numpy.isin

doc/whats_new.rst Outdated Show resolved Hide resolved
@gtdang
Copy link
Collaborator

gtdang commented Jun 20, 2024

@ntolley Seems like all tests passed except the linux. Shall we merge this so @kmilo9999 can continue testing the CI updates?

@jasmainak
Copy link
Collaborator

Just one tiny comment, otherwise good to merge.

ntolley and others added 2 commits June 20, 2024 12:56
@ntolley
Copy link
Contributor Author

ntolley commented Jun 20, 2024

Just one tiny comment, otherwise good to merge.

sounds good! everything should be addressed, whoever seeing the checks (besides ubuntu) go green feel free to merge

@jasmainak jasmainak merged commit 3ada0ff into jonescompneurolab:master Jun 20, 2024
4 of 9 checks passed
@jasmainak
Copy link
Collaborator

Thanks @ntolley !! 🥳

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.

5 participants