MAINT, TST: Fixes for Python 3.7 - #10740

I'm a little worried about the meaning of len here. Can you add a test for setting a (np.void, 7) with a ctypes.c_uint16.__ctype_le__ * 2 * 2 array?

I believe it is the same as it was before, just differently expressed. I'm going off the Python3.7 implementation of the previous functions. What are your concerns?

I incorrectly assumed that len was shape[0]. It would still be nice to add the ctypes test, but I guess out of scope.

Can you move these into {} blocks by the other #if defined?

What happens if you just always declare this const?

Fails. I tried that earlier, and const char * warns on assignment to the docstring. The type of the latter seems to have also been changed in Python 3.7 with the addition of const.

Could also fix this with a cast at the assignment location, but I guess fine to leave it.

Same remark about scoping this to where it's used, to remove an #if defined

I considered that, but some of the places would require free floating blocks, which I really don't like. I don't see much gain here, especially as this will all get cleaned up later when 2.7 is dropped.

Note that we will also be moving to C99 at some point after Python 2.7 and 3.4 are dropped, and it allows declarations after statements. Although, in cases like this, that probably just makes the code more difficult to understand as one never knows where to find the declaration...

Some advantages of floating blocks:

• They rule out places where the variable can be used
• When we switch to C99, the braces around them can be removed
• They reduce stack usage, as the variable does not consume the stack for the whole function

Still don't like floating blocks, haven't since Algol-68. If you really need them, you probably need a subroutine.

Fair enough

I know it's not necessary, but I'd find this clearer with a | PyBUF_SIMPLE here

PyBUF_SIMPLE is just zero. The documentation already sets out the hierarchy, the macros are not orthogonal.

I'd argue that SIMPLE is orthogonal to WRITEABLE, it just happens (/ is designed) that it's representation is 0.

Seems reasonable. I'll add when we decide about the comment for PyBuffer_Release

I don't understand the 3.7 behavior here. Can you comment the -1 case too?

AFAICT, the old Python3 behavior was to make an entry in the __warningregistry__ in all the modules up the line from the module in which the warning was issued, in 3.7 it looks like it is only in the module in which raised, hence the distinction of other. The -1 is generic to Python 3, as a version entry is added to all the created registries. Note that some of our functions clear the registries completely, which really isn't quite right for Python3. The tests are a bit of a kludge, and hard to maintain as the tested functions fiddle with private Python matters, but they do seem to work as advertized. These aren't my functions though, maybe @seberg can add more.

Yeah, the module warning filter is a bit strange (and possibly not as reliable as it could be). The bigger user is the clear_and_catch_warnings which I do not think we use anymore likely (so the behaviour tested is likely used in a single place and should not even matter, luckily).

The -1 is of course due to the "version" entry which was added in 3.6 and allows picking up changes in the filter list, which used to be very broken.

The change in py 3.7 is a bit unclear to me right now (I really should run a python3.7). I would expect something different going on, possibly:

1. >=Py3.6 adds the "version" information (to get rid of old activated filters
2. This was broken in the sense of breaking "once" filters to possibly occur more then once, because the __warningregistry__ was completely cleared!
3. So Py3.7 fixes it (somewhat, I expect it can't be gotten 100% right really), so that "once" or "ignore" filters will stay in the __warningregistry__ if there is still a matching filter active (which is the case here).

I expect "other" might want need to use the py2 behaviour here (not quite right probably too). I expect fully right would be something like:

a) "number of old outdated filters"
b) "number of old still active filters" (EDIT: was "already", "still" is correct)
c) "number of new filters"

Then Py2, will be (a + b + c), py3.6 ("version", but not 3.7) will be just c and py3.7 will be b+c, add -1 if "version" exists (py >3.6).

I have not checked it because I have no py3 install right now.

Sorry, b) would be: Number of old still (apparently) active filters

We could make other an n_still_active or so, and a number of all old filters so to not change the values in old py versions. The n_py3... is actually c, the n_in_context is actually a+c. Could also add the parameter: number of non-surviving old filters=0, since I bet 0 might work in all current places?

Ok, I hope this was understandable, if someone wants a quick test:

import warnings

# Distinguish python 2 from python 3.7:
# This warning should remain in the __warningregistry__ in python <3.5, but not in python 3.7 (or 3.6)
warnings.simplefilter('ignore', RuntimeWarning)
warnings.warn('A warning we will not ignore in the future!', RuntimeWarning)
warnings.simplefilter('always', RuntimeWarning)  # The last warning should be shown again (in principle)

print('Should have 1 warning (for all python versions):\n')
print(__warningregistry__)
print('\n-------------------\n')

warnings.warn("Hahahah!")  # Warning shown once only
print('\n< Py3.5 should have two filters (the old one survives, even though it is gone in principle), Py 3.6 and 3.7 should have one filter:\n')
print(__warningregistry__) 
print('\n-------------------\n')

# "Should" do nothing, but (partially) invalidates `__warningregistry__`
warnings.simplefilter('ignore', RuntimeWarning)  

warnings.warn("And another one.")
print("\nNow <Py 3.5 should have 3 filters `a+b+c`, Py=3.6 should have 1 (`c`) and Py3.7 should 2 (`b+c`)\n")
print(__warningregistry__)

EDIT: Fixed some stuff, also Python 3.5 alreadz has the "version" info, probably 3.4 does not, but might as well compare with Python 2.7...

Tricky stuff… I think that for Python 3.5+, as long as you make sure to never have active "once" filters, testing is actually pretty warning clean even without suppress_warnings, but oh well, it has a bit of other niceties to offer additionally. Maybe at some point we only support those and pytest might add some nice things and we can dump it again.

Mostly looking through the PR out of curiosity - and this comment confused me as PyObject_AsWriteBuffer is deprecated in python3; maybe clarify that the "The release below is unsafe, but the python2 PyObject_AsWriteBuffer does it as well.... etc."

As above; here, the comment is even more confusing for those who don't think about python2 (just imaging all python2 stuff gone - this comment will probably not get cleaned up unless we state python2 in it!)

This is only for Python 3. The PyObject_AsWriteBuffer/PyObject_AsReadBuffer functions currently available in Python 3 deal with a completely different buffer type than in Python 2, and it is only with the Python 3 Py_buffer that the reference to the supplying object can be released. AFAICT, the released reference is entirely useless for NumPy as ndarrays can't use it, so anything passing a buffer to an ndarray needs to know that. Maybe we should just say that we release the reference because there is nothing else to do with it. @pv Thoughts?

Do you mean AsReadBuffer? Are you saying that PyBuffer_Release is unsafe too, or just that AsReadBuffer is?

That should have been changed. Both release the buffer.

Can you add n_warns = len(mod_warns) - 1 at the top of the 'version' in mod_warns if, and then use that below? That makes the accounting for version appear in only one place.

I rewrote the whole bit to be simpler. Added two keywords py34 and py37 to account for the version dependent changes. Python 3.7 does not make entries for ignored warnings, so I suspect we will no longer need suppress_warnings when we drop support for earlier Python versions.

Is this comment still in the right place?

Beats me, it's not my comment and I don't really know what it means :-)

I don't know what it means either, but hoped you might :)

The mutual exclusion is a little clearer as:

if sys.version_info[:2] >= (3, 7):
    if py37 is not None:
        n_in_context = py37
elif sys.version_info[:2] >= (3, 4)
    if py34 is not None:
        n_in_context = py34

but generally much clearer than what was here before

I deliberately chose not to do it that way because:

• This may not be the last change
• I liked having all the versions laid out

I don't know what you mean by either of those points:

• Are you suggesting py3.8 may change it again? It's easier to chain anif onto my version than onto what this patch has
• Doesn't my version lay out the versions more clearly? The python versions aren't tangled between branches this way