Repository navigation
MAINT: Tidy PyArray_DescrConverter - #15265
Conversation
cb36aea to
7c21481
Compare
|
Will add this to my pile of "things which didn't work today"... Splitting the first two commits to separate PRs (gh-15266, gh-15267), Edit: Turns out all three pieces were orthogonal anyway, this PR now represents the third piece which doesn't depend on the first two. |
The `goto finish` part of this if was useless, as none of the steps in `finish` were ever executed.
7c21481 to
60f94e2
Compare
eric-wieser
changed the title
seberg
left a comment
seberg
left a comment
There was a problem hiding this comment.
LGTM, nice moving around much easier to read broken apart. The last if would be nice to remove, since you removed one further down? The other things I do not care about.
This makes the control flow easier to reason about - notably, the only case that now falls through the `if` / `else` are `bytes` objects.
`PyBytes_Check(obj)` was always true in these if statements
The string parsing is split all over the place in PyArray_DescrConverter. This commit brings all of it into one place, which makes `PyArray_DescrConverter` far easier to follow.
60f94e2 to
5da2c01
Compare
seberg
left a comment
seberg
left a comment
There was a problem hiding this comment.
Great, thanks. I think one of the declaration fixups came in the wrong commit, but I don't mind. I will merge this as soon as the tests all pass I think, since it will create merge conflicts with the Py3K cleanup, but it has to happen one way or the other...
Which one? |
|
6d2faa6 commit, the removal of the duplicate |
Nope, that's actually a bug in that intermediate commit. Happy to fix if you're happy to wait for CI again (or accept an empty diff between revisions as proof the rebase is safe) |
|
OK, I don't mind waiting, but really a nitpick IMO. |
We already converted the pyobject to a string on a previous line, there's no need to do it again.
5da2c01 to
ac44095
Compare
|
Proof the commits are the same, and then hopefully that there's no need to wait for CI: https://github.com/numpy/numpy/compare/5da2c01a7ae3b9fc3701801ab16d234e5425436c..ac44095f8f8aca2574091bb32d6328d0b6bbf29a |
|
CI passes, and this is an identical revision to the approved one, merging... |
2 participants
First part of some major cleanup to make
PyArray_DescrConvertermanageable.Likely easiest to review commit-by-commit.
Follow-up will be to replace the clumsy
NPY_FAIL/*at = NULLstuff internally to just use a regular return value, and writePyArray_DescrConverteras a thin wrapper.