DEP: Complete deprecation of invalid array/memory order - #14596
Conversation
There was a problem hiding this comment.
I think for errors I prefer the formatting:
PyErr_SetString(PyExc_ValueError,
"this is a very long error message .... ... ... .. "
"...");
As for the above comment, I do not think we typically put that in, not that it hurts, though, hmmm. The idea of the DEPRECATED comment is to find these places that need updating easier. What is important here is only the release note.
There was a problem hiding this comment.
I updated the PyErr_SetString, I'm happy to
- remove the both dates
- consolidate them into a single line
- /* more order formats were handled before DEP: Complete deprecation of invalid array/memory order #14596 */
There was a problem hiding this comment.
Others may disagree, but if you do not mind, lets just remove the comments, they are not interesting internally for numpy, only for downstream. And downstream would read the release notes.
Can you reformat the other error setting as well?
The main thing to do is add the release note snippet, thanks!
There was a problem hiding this comment.
Removed comments and added a release note snippet.
There was a problem hiding this comment.
Thanks. LGTM. Do you want to squash it into a single commit with DEP: ...? I can also squash merge, but thought I would mention just in case, since we slightly prefer merges.
There was a problem hiding this comment.
I'm happy to do what you prefer.
I'm squashing and force committing a single commit with the correctish title
|
Interesting. The test failures seem real. There must be some documentation around which actually uses the deprecated version, hah... |
f1c9fc3 to
093bc62
Compare
Fixed order=FORTRAN in docs Added release snippet
093bc62 to
6177445
Compare
seberg
changed the title
|
There's one test failure in windows for |
|
It's a Heisenbug that shows up randomly, usually on windows 32 bits. |
|
Hmmm, there once was a bug with EDIT: Anyway, made a PR for OpenBLAS, if we are lucky, the next update fixes it, I guess... |
|
Thanks @sethtroisi |
| # invalid after gh-14596 | ||
| for order in ['Z', 'c', False, True, 0, 8]: | ||
| x = np.array([[1, 2, 3], [4, 5, 6]], np.int32) | ||
| assert_raises(ValueError, x.flatten, {"order": order}) |
There was a problem hiding this comment.
This test is unfortunately nonsense, this passes a dict as the order argument.
5 participants
Original deprecation was submitted in #6823 in 2015.