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

updating numpy + vtk functions #46

Merged
merged 21 commits into from
Oct 3, 2024
Merged

Conversation

AurelienBesnier
Copy link
Contributor

Hi !
I updated some of the imports of vtk modules which were outdated and also made some updates of numpy functions to be compatible with 2.0 and 1.x.

For the compatibility with numpy 2.0 we still need to wait for the skan package to be updated.

Also with @christian34 we investigated the problems we had with the CI and they are now fixed.

@pradal pradal self-requested a review October 2, 2024 19:56
@pradal
Copy link
Contributor

pradal commented Oct 2, 2024

Excellent.
You introduce a new dependency : vtkmodule.
Is it provided by vtk?
Otherelse you have to add it to meta.yaml

Copy link
Contributor

@pradal pradal left a comment

Choose a reason for hiding this comment

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

This sound great!
Thank you for modernize the code
Just remove cython if not used anymore

- {{ PYTHON }} -m pip install . -vv

requirements:
host:
- python
- setuptools
- numpy
- scipy
- cython
Copy link
Contributor

Choose a reason for hiding this comment

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

If you completly remove cython, remove it from the deps too

Copy link
Contributor

Choose a reason for hiding this comment

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

I do not think that we want to remove cython, as we want the two c extensions for acceleration !

Copy link
Contributor

Choose a reason for hiding this comment

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

I misunderstood some commit. I was a little bit afraid...

@AurelienBesnier
Copy link
Contributor Author

The vtkmodules is included in the vtk package. As per the vtk documentation:

The 'vtk.py' module is obsolete and is only provided for backwards compatibility. When you import vtk, this module actually provides you with the 'vtkmodules.all' module via some internal trickery:

>>> import vtk
>>> vtk.__name__
'vtkmodules.all'

@pradal pradal merged commit 50918da into openalea:master Oct 3, 2024
1 check passed
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.

3 participants