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

[FIX] issue #29 #34

Merged
merged 1 commit into from
Apr 8, 2024
Merged

[FIX] issue #29 #34

merged 1 commit into from
Apr 8, 2024

Conversation

Katterrina
Copy link

I had the same problem as mentioned here. According to NumPy 1.20.0 Release Notes, changing np.float to float should work.

I think it is better to change every occurrence of np.float (and also np.int, np.bool and np.object, because they are also deprecated) in the package, not only mesh_elements.py, because it will cause problems in the other places as well. I went through all the occurrences and tried to do the changes.

I was not sure about some decisions base.py. In this file, np.float is always used as dtype in np.loadtxt function. It is not necessary to specify dtype=float in this function because it is the default option (https://numpy.org/doc/stable/reference/generated/numpy.loadtxt.html), but it might be useful for readability, so I changed np.float to float everywhere in this file. There are also occurrences of np.loadtxt function with dtype='str'. I did not want to mix two types of dtype settings (string and python type) and this is the only place using string, so I changed dtype='str' to dtype=str.

deprecated numpy types changed to Python types
(https://numpy.org/devdocs/release/1.20.0-notes.html)
@Katterrina
Copy link
Author

I wanted to test my changes using pytest --doctest-modules enigmatoolbox as mentioned in contributing guidelines, but I always got ModuleNotFoundError: No module named 'vtk'. When I tried to install vtk using pip, it said Requirement already satisfied, so I am not sure what is wrong and I was not able to run the tests properly.

@kaurao
Copy link

kaurao commented Apr 5, 2024

This PR fixes is quite a reoccuring and annoying issue. Will be great if its merged. Thanks!

@saratheriver saratheriver merged commit 2591cce into MICA-MNI:master Apr 8, 2024
@saratheriver
Copy link
Collaborator

Done! Apologies for the delay & thanks for contributing.

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.

None yet

3 participants