-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Cast to integer to get rid of numpy warning #3241
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
Cast to integer to get rid of numpy warning #3241
Conversation
Can you please squash the commits? If you've never done this, you can ask one of the git guru experts. |
Given the nature of these changes I think it is better to leave this un-squashed to make bisect work well. |
These are all integer casting. If a bug occurs, it should be pretty straightforward to identify which line it comes from. |
@NelleV I discussed this with @tacaswell The issue is that these are all over the code base in widely different parts of the codebase. |
I guess this is a philosophical questions, but I am afraid we are going to end up with twenty of these one liner commits. They all solve one problem, thus could be in a single commits. Moreover, the commits messages makes it really hard to distinguish between all of these (thus the squash comment). |
This is not a case of that I don't want to rebase we just don't think that it is the right solution. Rewrote to use better commit messages. |
Fixes numpy issues warnings
Fixes numpy warnings in indexing
as suggested by comment
The contour.py change in this PR tries to int-ify things that are compared vs. nan for control flow later. Also I think the cbook.py int-ification is redundant with a change later in that function which could be removed. Anyway I agree with the idea of making changes like these, and I also did most of them in #3289 before I found this PR. I didn't do the ones in hatch.py because they could be more subtle. I don't care which PR is accepted or how the commits are squashed or separated.. |
This is to allow checking againt nan below Thangs @argriffing
Thanks @argriffing You are right. I did not look closely enough at that piece of code but trusted the comment above the line about casting. I have updated this pull request with your fixes. The fact that this didn't fail indicates that we are missing a unit test to cover these cases. This would be good to add if possible. This pull request fixes all the warnings that are raised during the test suite. However, it is not unlikely that there are more in parts of the code not unit tested. If anyone spots them they should be reported at the Github bug tracker. I guess we will only really know when numpy starts turning these into real errors. Why do you think the changes in hatch.py ar subtle? One can delay the cast until the actual indexing but it seems to me that the number of lines must be an integer pr definition. |
You are probably right. Everything is much more straightforward when you believe comments and variable names! |
Closes #3137 |
Cast to integer to get rid of numpy warning
Cast to integer to get rid of numpy warning
cherry-picked as 3bc07d1 |
No description provided.