Skip to content

Weird dtype->f->(scanfunc|fromstr) inconsistency #14173

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.

Sign up for GitHub

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

Jump to bottom
Closed
zjpoh opened this issue Aug 1, 2019 · 1 comment · Fixed by #14174
Closed

Weird dtype->f->(scanfunc|fromstr) inconsistency #14173

zjpoh opened this issue Aug 1, 2019 · 1 comment · Fixed by #14174

Comments

@zjpoh
Copy link
Member

zjpoh commented Aug 1, 2019

I found the following inconsistency in PyArray_FromString.

In the following line we check dtype->f->scanfunc == NULL to determine if the C function to read from string is defined.

numpy/numpy/core/src/multiarray/ctors.c

Line 4003 in ee309d9

if (dtype->f->scanfunc == NULL) {

If dtype->f->scanfunc != NULL, then array_from_text is called with fromstr_next_element to read the next element from string.

numpy/numpy/core/src/multiarray/ctors.c

Lines 4017 to 4021 in ee309d9

ret = array_from_text(dtype, num, sep, &nread,
data,
(next_element) fromstr_next_element,
(skip_separator) fromstr_skip_separator,
end);

However, the actual function that read from string is dtype->f->fromstr, not dtype->f->scanfunc.

numpy/numpy/core/src/multiarray/ctors.c

Lines 47 to 51 in ee309d9

fromstr_next_element(char **s, void *dptr, PyArray_Descr *dtype,
const char *end)
{
char *e = *s;
int r = dtype->f->fromstr(*s, dptr, &e, dtype);

This doesn't cause any breaking problem because, in the code, if dtype->f->scanfunc is defined then dtype->f->fromstr is also defined. However, I think it's easier to maintain if the check is checking the actual function.

Thoughts?
@seberg
Copy link
Member

seberg commented Aug 1, 2019

Sounds like a copy paste error from 20 years ago, PRs welcome.

@zjpoh zjpoh mentioned this issue Aug 1, 2019
Merged
seberg pushed a commit that referenced this issue Aug 2, 2019
@zjpoh @seberg
BUG: Check for existence of fromstr which used in `fromstr_next_ele…
3d5fee2 
…ment` (gh-14174)

The check tested the wrong function. In principle a dtype could only implement one of the two slots/functions.

Fix #14173.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

BUG: Check for existence of fromstr which used in fromstr_next_element zjpoh/numpy
2 participants
@seberg @zjpoh