cc @scoder

Don't we have any tests for .pxd files? We do have a USE_SDIST CI job, but maybe it doesn't intersect with a test if we have one?

Don't we have any tests for .pxd files?

Probably difficult to test the integration here because Cython would always find a numpy/__init__.pxd files, even if it uses its own one.

The problem is not with the USE_SDIST job, since the pxd was shipped in the sdist tarball. It was not shipped in the regular wheel. Should I add a CI step specifically to test that the files in the wheel match expectations? What other files do we expect to be there?

This could be a backport candidate to 1.18? In the spirit of maximising compatible versions / minimising pitfalls once cython 3.0 is out?

backport … minimising pitfalls once cython 3.0 is out

Cython still ships its own NumPy .pxd file, so users won't end up without one.

If NumPy 1.18/9 starts providing its own version, then Cython should prefer that, and the one in Cython will only be used e.g. if NumPy is not installed or too old. But that case might not be obvious to users, so we should make sure that both are aligned for the time being.

AFAICT, both are the same in the Cython 0.29 series, and the only difference in Cython 3.0 is that the struct fields ndim, shape, strides, size and data of the ndarray type have been replaced by properties that use the corresponding C-API functions instead of direct struct field access.

So, depending on which version is used, the generated code will end up with either struct field access or API calls. Sadly, the version that is now in NumPy uses the (deprecated) direct field access, and switching to the version in Cython 3.0 requires … Cython 3.0, on user side.

To me, this suggests (I think) that it might actually be better for NumPy to not ship the .pxd file in 1.18, so that Cython 3.0 users get API calls into their code (and 0.29.x users won't), and once Cython 3.0 is more widely in use, NumPy can switch to requiring its use and ship its own maintained version of the .pxd, which will then be preferred over the one in Cython.

Should I add a CI step specifically to test that the files in the wheel match expectations? What other files do we expect to be there?

Maybe it would make more sense to test the wheels wherever they are produced?

The problem is not with the USE_SDIST job, since the pxd was shipped in the sdist tarball. It was not shipped in the regular wheel

That's not what I meant. The problem shows up in both USE_SDIST and USE_WHEEL, the point of those jobs is to test a numpy installed from release artifacts. Right now both sdist and wheel are broken, for the same reason: the .pxd does not get installed.

Ideally what you want to test is not only that the file gets installed, but also that it's not broken.

FYI, I think (see cython/cython#3573) we will start allowing a versioned naming scheme for .pxd files to prevent older Cython versions from running into files that use a newer syntax. That would allow NumPy to immediately start shipping declaration files that make use of newer syntax features for Cython versions that support it.

@scoder
Thanks for the enlightening explanation. A version scheme seems to make a lot of sense!

Ideally what you want to test is not only that the file gets installed, but also that it's not broken.

We do test cython in the random api cython tests. We could add a marker like in the recent addition to the pxd and make sure the C file includes it, to verify our version of the pxd is used. This test_cython test is run in the USE_WHEEL and RUN_FULL_TESTS travis run.

@scoder I would like to ship the identical version in 1.19 just to be absolutely sure that there is no problem with it. I was hoping that would have happened already for 1.18, but the bug fixed here prevented that.

We could add a marker like in the recent addition to the pxd and make sure the C file includes it, to verify our version of the pxd is used.

that sounds like a good idea

ship the identical version in 1.19

Which version? The one that is currently in NumPy? Or did you mean to make sure both Cython (0.29) and NumPy come with the same version?

I added a couple of nogil markers in Cy3.0 to the properties that (supposedly) needed them. The attributes didn't need them before, but I'd like to be sure that nogil access is something that NumPy can commit to.

The one that is currently in NumPy

Yes: the one mentioned in NumPy 1.18 is meant to be functionally the same as the one in Cython 0.29. This PR contains a test to make sure NumPy is using its version, which is heavily used in numpy.random.

As for nogil, yes that's fine. The generated code is intended to behave exactly the same before and after the transition to getters: direct field access in C. The 0.29 Cython
used direct access via struct fields with some aliasing, the 3.0 Cython uses PyArray_XXX macros. The end result is meant to be exactly the same same, nogil, code.

Let's give this a shot. Thanks Matti.

Hmm, looks like another fixup is needed. See #12284 (comment) .