DOC: Add and fixup/move docs for descriptor changes by seberg · Pull Request #25946 · numpy/numpy

seberg · 2024-03-06T07:31:16Z

This is a "pre" PR for the actual changes in #25943. The point is that the PR breaks the docs build as it is enough of an ABI break that downstream has to recompile.

That will take a few days (maybe a bit longer if pyibind11 proofs problematic, although it seems problematic use of pybind11 is rare, e.g. in SciPy only one file/module was affected).

Thus, split it out so we can put it in (and also see doc mistakes).

jorisvandenbossche · 2024-03-06T08:11:43Z

doc/source/numpy_2_0_migration_guide.rst

+The ``PyArray_Descr`` struct has been changed
+---------------------------------------------
+One of the most impactful C-API changes is that the ``PyArray_Descr`` struct
+is no more opaque to allow us to add additional flags and have


Should this say it is now opaque instead of no longer opaque? Unless I am misinterpreting the word opaque, but I would assume you want to make it more opaque to allow changing it later without breaking user code again

Thanks, it was meant to be a "now", not a "no". It is mostly changed really, but I think it makes sense to write it as opaque.
(I.e. elsize is available if you set -DNPY_TARGET_VERSION=NPY_2_0_API_VERSION, but that should be relevant only for authors of new-style DTypes.)

rgommers · 2024-03-06T15:31:04Z

Looks like a good idea to split this off, to keep PR sizes reasonable. This already accumulated a conflict in c-api/array.rst.

[skip actions] [skip cirrus] [skip azp]

seberg · 2024-03-06T16:08:13Z

This already accumulated a conflict in c-api/array.rst.

Fixed. The important thing is I would like this merged before the other PR. Because the other PR will break doc builds (hopefully not for long), and we need the newest docs visible.

doc/source/release/2.0.0-notes.rst

mattip

LGTM. One typo that became a refactor.

Co-authored-by: Matti Picus <matti.picus@gmail.com>

mattip

Now that I looked at the actual rendered documentation, I see some problems

mattip · 2024-03-06T17:13:40Z

doc/source/numpy_2_0_migration_guide.rst

+
+Where not possible, new accessor functions are required:
+* ``PyDataType_ELSIZE`` and ``PyDataType_SET_ELSIZE`` (note that the result
+  is now ``npy_intp`` and not ``int``).


Formatting is off in the rendered page

Suggested change

is now ``npy_intp`` and not ``int``).

is now ``npy_intp`` and not ``int``).

mattip · 2024-03-06T17:15:10Z

doc/source/numpy_2_0_migration_guide.rst

+
+**Custom User DTypes:**
+Existing user dtypes must now use ``PyArray_DescrProto`` to define their
+dtype and slightly modify the code. See note in `PyArray_RegisterDataType`.


Link to PyArray_RegisterDataType is broken in the rendered page.

mattip · 2024-03-06T17:17:08Z

doc/source/reference/c-api/array.rst

+    is non- ``NULL`` (the fields member of the base descriptor can be
+    non- ``NULL`` however).
+
+    .. c:type:: PyArray_ArrayDescr


Is this indented on purpose? See the rendered page

mattip · 2024-03-06T17:18:35Z

doc/source/reference/c-api/types-and-structures.rst

+   Some dtypes have additional members which are accessible through
+   `PyDataType_NAMES`, `PyDataType_FIELDS`, `PyDataType_SUBARRAY`, and
+   in some cases (times) `PyDataType_C_METADATA`.
+


These reference links are not working. Do you need a domain?

mattip · 2024-03-06T17:19:50Z

doc/source/reference/c-api/types-and-structures.rst

+       See `PyDataType_ELSIZE` and `PyDataType_SET_ELSIZE` for a way to access
+       this field in a NumPy 1.x compatible way.
+
+   .. c:member:: npy_intp alignment


The links are broken. Do you need a domain?

mattip · 2024-03-06T17:20:05Z

doc/source/reference/c-api/types-and-structures.rst

-       An ordered tuple of field names. It is NULL if no field is
-       defined.
+       See `PyDataType_ALIGNMENT` for a way to access this field in a NumPy 1.x
+       compatible way.


The link is broken.

mattip · 2024-03-06T17:23:19Z

doc/source/release/2.0.0-notes.rst

+----------------------------------------------
+Unless compiling only with NumPy 2 support, the ``elsize`` and ``aligment``
+fields must now be accessed via `PyDataType_ELSIZE`,
+`PyDataType_SET_ELSIZE`, and `PyDataType_ALIGNMENT`.


The links above are broken. Maybe you can fix the other warnings in the release notes, see the warnings starting here

ngoldbaum

spotted a couple typos

ngoldbaum · 2024-03-06T17:28:00Z

doc/source/numpy_2_0_migration_guide.rst

+One of the most impactful C-API changes is that the ``PyArray_Descr`` struct
+is now more opaque to allow us to add additional flags and have
+itemsizes not limited by the size of ``int`` as well as allow improving
+structured dtypes in the future and not burdon new dtypes with their fields.


Suggested change

structured dtypes in the future and not burdon new dtypes with their fields.

structured dtypes in the future and not burden new dtypes with their fields.

ngoldbaum · 2024-03-06T17:28:28Z

doc/source/numpy_2_0_migration_guide.rst

+structured dtypes in the future and not burdon new dtypes with their fields.
+
+Code which only uses the type number and other initial fields is unaffected.
+Most code will hopefull mainly access the ``->elsize`` field, when the


Suggested change

Most code will hopefull mainly access the ``->elsize`` field, when the

Most code will hopefully mainly access the ``->elsize`` field, when the

mattip · 2024-03-06T18:08:42Z

Lets merge this now and work on cleanups later.

seberg · 2024-03-06T18:18:09Z

Thanks, one additional small fixup, will be to add the additional reserved_null field, we discussed.

…elsize (#40418) ### Rationale for this change NumPy 2.0 is changing some ABI, see the issue description and numpy/numpy#25946 for more details. The changes here should make our code compatible both with current numpy 1.x and future numpy 2.x * GitHub Issue: #40376 Lead-authored-by: Joris Van den Bossche <jorisvandenbossche@gmail.com> Co-authored-by: Sebastian Berg <sebastianb@nvidia.com> Signed-off-by: Joris Van den Bossche <jorisvandenbossche@gmail.com>

github-actions bot added the 04 - Documentation label Mar 6, 2024

jorisvandenbossche reviewed Mar 6, 2024

View reviewed changes

seberg force-pushed the descr-abi-break-docs branch 3 times, most recently from fcbbab1 to 0f987a8 Compare March 6, 2024 08:16

DOC: Add and fixup/move docs for descriptor changes

7653a32

[skip actions] [skip cirrus] [skip azp]

seberg force-pushed the descr-abi-break-docs branch from 0f987a8 to 7653a32 Compare March 6, 2024 16:07

mattip reviewed Mar 6, 2024

View reviewed changes

doc/source/release/2.0.0-notes.rst Outdated Show resolved Hide resolved

mattip approved these changes Mar 6, 2024

View reviewed changes

DOC: update doc/source/release/2.0.0-notes.rst

00f8641

Co-authored-by: Matti Picus <matti.picus@gmail.com>

mattip reviewed Mar 6, 2024

View reviewed changes

ngoldbaum reviewed Mar 6, 2024

View reviewed changes

mattip merged commit ae8f784 into numpy:main Mar 6, 2024

mattip mentioned this pull request Mar 6, 2024

DOC: fixup new doc build warnings after PR 25946 #25951

Closed

This was referenced Mar 8, 2024

GH-40376: [Python] Update for NumPy 2.0 ABI change in PyArray_Descr->elsize apache/arrow#40418

Merged

API: Restructure the dtype struct to be new dtype friendly #25943

Merged

	is now ``npy_intp`` and not ``int``).
	is now ``npy_intp`` and not ``int``).

	structured dtypes in the future and not burdon new dtypes with their fields.
	structured dtypes in the future and not burden new dtypes with their fields.

	Most code will hopefull mainly access the ``->elsize`` field, when the
	Most code will hopefully mainly access the ``->elsize`` field, when the

Uh oh!

Comments

Conversation

seberg commented Mar 6, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rgommers commented Mar 6, 2024

Uh oh!

seberg commented Mar 6, 2024

Uh oh!

Uh oh!

mattip left a comment

Choose a reason for hiding this comment

Uh oh!

mattip left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ngoldbaum left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mattip commented Mar 6, 2024

Uh oh!

seberg commented Mar 6, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants