Modified iterator & counter type from int to Py_ssize_t throughout Matrix class #39965
Conversation
|
Documentation preview for this PR (built with commit 98eb7ad; changes) is ready! 🎉 |
|
when would this matters, the matrix size may exceed 2^31-1? |
|
I'm not completely sure why, other than it was proposed in the issue lol. I guess my only defense is that it is common practice in most of the sage framework to use Py_ssize_t for iteration counter & indices, and to use int and sage Integer for operations and storage |
There was a problem hiding this comment.
@user202729 That is my understanding. I think it also helps for some consistency too. At least, I don't see any harm in this. So I am setting this to a positive review. If you object, please feel free to revert.
|
Looks like we're not supporting matrices with 2^31 rows/columns anyway. But there shouldn't be any harm in this either. (there are some anecdote regarding |
#3505
This PR implements the mentioned improvement in the issue. Carefully only changed the int type for counter & iterator variable declarations, others were kept as is. Also, kept the size_t types as is (though I think some could also be changed to Py_ssize_t, depending on preference).
📝 Checklist
⌛ Dependencies
cc @tscrim