Skip to content

MAINT,API: Always export static inline version of array accessor.#25789

Merged
mattip merged 2 commits into
numpy:mainfrom
seberg:only-static-inline
Feb 8, 2024
Merged

MAINT,API: Always export static inline version of array accessor.#25789
mattip merged 2 commits into
numpy:mainfrom
seberg:only-static-inline

Conversation

@seberg

@seberg seberg commented Feb 8, 2024

Copy link
Copy Markdown
Member

While we might not hide the struct fully by default, I don't think there is a reason to not only export the static inline versions of these?

They will type check a bit nicer (might give warnings), but otherwise should only matter if you do PyArray_NDIM(arr) = 3.


Or am I missing a reason to even have this? Seems like a relatively mild and good API change to only use the static-inline versions.

While we might not hide the struct fully by default, I don't think
there is a reason to not only export the static inline versions of
these?

They will type check a bit nicer (might give warnings), but otherwise
should only matter if you do `PyArray_NDIM(arr) = 3`.

@Mousius Mousius left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Not entirely sure on the history here but this seems to be straightforward to me as well.

@mattip mattip left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM. One doc nit to make the intention clearer.

Comment thread doc/release/upcoming_changes/25789.c_api.rst Outdated
[skip cirrus] [skip azp] [skip actions]

Co-authored-by: Matti Picus <matti.picus@gmail.com>
@mattip mattip merged commit 8361485 into numpy:main Feb 8, 2024
@mattip

mattip commented Feb 8, 2024

Copy link
Copy Markdown
Member

Thanks @seberg

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.

3 participants