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

MAINT: Fixes for NumPy deprecation and 3.10 #42

Merged
merged 32 commits into from
May 9, 2022
Merged

Conversation

larsoner
Copy link
Collaborator

@larsoner larsoner commented May 3, 2022

  1. np.float is just an alias for float so avoid the deprecation warning about it.
  2. Change to manylinux2014 for Python3.10 support
  3. Add shims for VTK not having a 3.10 wheel (via wheels.pyvistia.org)

FYI it would be good to merge this and cut a release since there are no Python 3.10 wheels for pymeshfix.

@larsoner
Copy link
Collaborator Author

larsoner commented May 3, 2022

I have a feeling oldest-supported-numpy might work for us here (somehow), but I'm just bumping the requirements_build.txt here hoping things come back green

Comment on lines +47 to +52
- name: Manually install VTK wheel on 3.10 since it does not work locally
run: |
pip install --upgrade pip
curl https://wheels.pyvista.org/vtk-9.1.0.dev0-cp310-cp310-macosx_12_0_x86_64.whl -o vtk-9.1.0.dev0-cp310-cp310-macosx_10_16_x86_64.whl
pip install ./vtk-9.1.0.dev0-cp310-cp310-macosx_10_16_x86_64.whl
if: ${{ matrix.os == 'macos-12' }}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Locally on my 12.3 Monterey install I cannot reproduce this issue, but on GH actions there is some problem installing this wheel despite many attempts at debugging. So here I just hack the filename to make it seem like at 10_16 wheel so that pip will install it.

It's a hack but I think it's acceptable since we want a 3.10 wheel for this package, and this hack can go away once VTK releases 9.2 with 3.10 wheels in the coming weeks (!?)

@larsoner larsoner changed the title MAINT: Fix for NumPy deprecation MAINT: Fixes for NumPy deprecation and 3.10 May 3, 2022
@larsoner
Copy link
Collaborator Author

larsoner commented May 3, 2022

Finally green!

@larsoner
Copy link
Collaborator Author

larsoner commented May 6, 2022

@pyvista/developers this one is ready for review. It would be nice to merge this so that we could get 3.10 wheels on PyPi

pymeshfix/cython/_meshfix.pyx Outdated Show resolved Hide resolved
@larsoner
Copy link
Collaborator Author

larsoner commented May 6, 2022

... I went a step further and just called ascontiguousarray with a dtype, as it should essentially be a no-op / just create a view if it's already contiguous, e.g.:

>>> import numpy as np
>>> x = np.array([1.], np.float64)
>>> y = np.ascontiguousarray(x, dtype=np.float64)
>>> y[0] = 0
>>> x
array([0.])

I also fixed some plain assert statements to raise a proper error, and changed to bare raise Exceptions to raise RuntimeErrors

@adeak
Copy link
Member

adeak commented May 6, 2022

... I went a step further and just called ascontiguousarray with a dtype, as it should essentially be a no-op / just create a view if it's already contiguous, e.g.:

Yeah, that's what I had in mind.

I also fixed some plain assert statements to raise a proper error, and changed to bare raise Exceptions to raise RuntimeErrors

Good idea, thanks. I wonder if we should define some more meaningful errors for these... not sure it's worth it for purity's sake.

@larsoner
Copy link
Collaborator Author

larsoner commented May 6, 2022

Good idea, thanks. I wonder if we should define some more meaningful errors for these... not sure it's worth it for purity's sake.

Seems like overkill. I doubt users will hit these very often... but moving to a more specific builtin error type seemed like a minimal amount of effort required to make it slightly better here.

@larsoner
Copy link
Collaborator Author

larsoner commented May 6, 2022

All green again, okay to merge @adeak ?

@adeak
Copy link
Member

adeak commented May 6, 2022

Sorry, @larsoner, I can't review any of the CI changes :( The numpy part seems good to me.

@larsoner
Copy link
Collaborator Author

larsoner commented May 9, 2022

I think the CI stuff is necessary to get 3.10 support. If anyone else wants to look and finds problems, I'm happy to open another PR, or revert and try again! But in the meantime I'll go ahead and merge and cut a release to see if PyPi wheels show up.

@larsoner larsoner merged commit f06793b into pyvista:master May 9, 2022
@larsoner larsoner deleted the numpy branch May 9, 2022 15:12
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.

2 participants