-
-
Notifications
You must be signed in to change notification settings - Fork 10.8k
Ticket1676 - dtype problem with append_fields #22
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
Conversation
The patch looks ok to me. The function itself has a few inconsistencies though:
|
Thanks for the review. The hasattr(dtypes, 'iter'): looks like a potential bug, as iterables aren't guaranteed to support the len function. Tuple and list look like the right way to go, although one might argue for strings for setting names to ["a", "b",...]. That seems a bit a reach though. Other sequence types don't look appropriate. I've gone ahead and removed the None from the data and marked the dtype list as optional in the docs. I'll commit it in tomorrow morning after I get some coffee. You asked for commit rights a while back. Are you still interested? |
The changes looks good. Yeah, I'm still interested in the commit rights. |
Committed along with a regression test. |
…ialization Merge in numpy-hpy from ss/port-more-init to labs-hpy-port * commit '3cd9bf37f3355055482d0b3404189c606468f7bc': Remove debugging printf Add one more missing C API warning Remove busdaycalendar for the HPy example port multiarray mod init: convert setting of remaining types on module dict multiarray mod init: add C API warnings for unported parts multiarray mod init: convert setting of "flatiter" on module dict multiarray mod init: convert setting of "ndarray" on module dict multiarray mod init: do not leak the h_d handle multiarray mod init: convert datetime capsules creation multiarray mod init: convert API capsules creation multiarray mod init: convert HPyContext capsule creation Remove PyObject* for module from the multiarray module init Port initumath Convert set_flaginfo
feat: Add vzip_s8
This is the patch attached to ticket 1676, it needs review.