Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DOC: Fix warning for PyArray_MapIterNew. #24130

Merged
merged 2 commits into from
Jul 9, 2023
Merged

Conversation

liang3zy22
Copy link
Contributor

@liang3zy22 liang3zy22 commented Jul 6, 2023

Fix two warnings about missing reference . The two warnings are for PyArray_MapIterNew.

Copy link
Contributor

@rossbar rossbar left a comment

Choose a reason for hiding this comment

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

I think the main question here is whether PyArray_MapIterNew is intended to be public C-API. From the references in the Advanced Indexing doc which you pointed out, it seems plausible that it is intended to be public. @seberg / @charris any thoughts on this one?

If this is a public function then this change LGTM, with perhaps a slight update to the c:function description to be a little more specific.

@seberg
Copy link
Member

seberg commented Jul 8, 2023

I would like to make it proper private and see if someon complains, the only known user was theano, which I consider historic at this point. For practically everything getting it right and fast is difficult, so using advanced indexing or ufunc.at is better anyway (including the theano usecase).

Keeping it public makes the struct a big mess, so I have doubts anyone figured out how to use it the fast way.

@rossbar
Copy link
Contributor

rossbar commented Jul 8, 2023

Thanks @seberg ! Given that, @liang3zy22 I think a better approach would be to convert the references to PyArray_MapIterNew to unlinked objects (e.g. use PyArray_MapIterNew instead).

@rossbar
Copy link
Contributor

rossbar commented Jul 9, 2023

Rendered docs look good, in it goes. Thanks @liang3zy22 !

@rossbar rossbar merged commit 1222ed5 into numpy:main Jul 9, 2023
51 of 55 checks passed
@liang3zy22 liang3zy22 deleted the gh13114 branch July 9, 2023 08:12
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.

None yet

3 participants