This comment would seem helpful only if you thought StructureFormat should be used elsewhere (as it was originally, but which is not all that obvious). Maybe just state here something along the lines of "'void' only deals with unstructured bytes; we use StructureFormat if they are assigned to fields. Note that we cannot use StructureFormat where the void is in a union with a non-void dtype; those will be caught above and the typeset with the regular dtype (though the full dtype will be printed, to indicate to the user it is a union dtype)"

If you thought StructureFormat should be used elsewhere

That depends on what you think counts as a structured type:

• issubdtype(dt, np.void) and dt.names is not None
• dt.names is not None
• issubdtype(dt, np.flexible) (a lot of old code refers to flexible types when it means structured types)

Renaming it to StructuredVoidFormat would make that clear

Agreed it is confusing, but this is one place where one can explain what it is in the code!

As we're adding comments, maybe here note that the latter part guards against union dtypes?

Might as well test the output?

Output is meaningless with __module__ assigned to garbage, I think. Can __module__ ever be None in real code?

Yes, true. I tried it myself and indeed got __main__.CustomScalar - I just wondered whether this is tested anywhere at all...

More background behind this comment would be useful. Can this ever happen?

Hmm my memory of this is vague now. I think this code had to do with "old style" vs "new style" classes in python2. I think "old style" classes fill in the __module__ entry incompletely when defined in C, or something along those lines. I will have to look it up.

Ah, happily I left some kind of explanation of that in a github comment. See here (link).

Now as to whether this can be NULL, I think it can, for types defined in C in python 2. If you go to https://docs.python.org/2/extending/newtypes.html, and copy the first "noddy" example, you will find:

>>> import noddy
>>> mynoddy = noddy.Noddy()
>>> mynoddy.__module__
AttributeError: 'noddy.Noddy' object has no attribute '__module__'
>>> import cPickle
>>>  cPickle.dumps(mynoddy)
TypeError: can't pickle Noddy objects

So, should the code continue to be able to deal with __module__ == NULL? Or is the new behaviour of raising an exception fine? It would probably be good to add a comment with the conclusion regardless...

It's probably fine to raise an exception if it is NULL, let's go with these changes.I would change the comment, though, it is technically wrong: Instead of this should never happen since types always have these attributes it should say something like we do not expect this to happen happen since types always have these attributes, except in some very special cases (module may be null for "statically allocated" types in python2).

Probably these "special cases" never come up in practice, since they will cause people other problems (eg, unpicklable types).

One last comment: I looked more closely at this and I have not been able to construct an example where __module__ is NULL. The type from numpy.core.test_rational import rational is what I expected to fail, since it does not properly define a module, but if you do rational.__module__ you get __builtin__, which is not NULL. I tested that np.dtype((rational, 'i4,i4')) works properly with this PR.

So in summary, this code is fine.

Should we add a deprecation warning for use of StructureFormat, the same way we did for LongFloatFormat?

Sure, wouldn't hurt

Some of these lines are too long, too