Skip to content

MAINT: Move np.dtype.name.__get__ to python #11932

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

Merged
merged 1 commit into from
Sep 13, 2018

Conversation

eric-wieser
Copy link
Member

@eric-wieser eric-wieser commented Sep 12, 2018

This shares a lot of code with part of dtype.__repr__, and it's helpful to get it all in one place.

This fixes possible but unlikely segfaults if PyUString_ConcatAndDel fails.

Follows up on some remarks left in #10602, hopefully makes it easier to solve problems in #10151.

res = PyUString_FromString(typeobj->tp_name);
}
else {
res = PyUString_FromStringAndSize(s + 1, strlen(s) - 1);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This entire block is just typeobj.__name__

return dtype.type.__name__

# Builtin classes are documented as returning a "bit name"
name = dtype.type.__name__
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line blocks #10151, but so did the equivalent C code


# append bit counts to str, unicode, and void
if np.issubdtype(dtype, np.flexible) and not _isunsized(dtype):
name += "{}".format(dtype.itemsize * 8)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@charris: As predicted, the assumption that a byte is 8 bits is pretty ingrained into our dtype string handling

This shares a lot of code with part of `dtype.__repr__`, and it's helpful to get it all in one place.

This fixes possible but unlikely segfaults if `PyUString_ConcatAndDel` fails.
const int np_prefix_len = sizeof(np_prefix) - 1;
PyTypeObject *typeobj = self->typeobj;
/* let python handle this */
PyObject *_numpy_dtype;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be static PyObject *_numpy_dtype = NULL; It needs to stay around and needs to be initialized.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it does need to stay around, and it's initialized at its first use below.

This matches dtype.__repr__, in that both do not bother to cache the module or the function

@charris charris merged commit ed12495 into numpy:master Sep 13, 2018
@charris
Copy link
Member

charris commented Sep 13, 2018

Thanks Eric.

@charris charris changed the title MAINT: Move np.dtype.name.__get__ to python MAINT: Move np.dtype.name.__get__ to python Nov 10, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants