Skip to content

DEP: deprecate scalar conversions for arrays with ndim>0 #21846

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

Closed
wants to merge 41 commits into from

Conversation

Micky774
Copy link
Contributor

@Micky774 Micky774 commented Jun 24, 2022

Related Issues/PRs

Resolves #10615 (stalled)

Fixes #10404

Changes

PR #10615: Raises deprecation warning for automatic conversion of size=1 ndarray instances to numpy scalars, e.g.

a = np.array([3.14]) #deprecated
b = np.array([[3.14]]) #deprecated
c = a[0] #preferred

This PR: Reconciles main and fixes tests.

Comments

Co-authored by @nschloe

@Micky774 Micky774 changed the title Scalar conversion DEP: deprecate scalar conversions for arrays with ndim > 0 Jun 24, 2022
@Micky774 Micky774 changed the title DEP: deprecate scalar conversions for arrays with ndim > 0 DEP: deprecate scalar conversions for arrays with ndim>0 Jun 24, 2022
@mattip
Copy link
Member

mattip commented Jul 12, 2022

If I recall correctly the ArrayAPI always requires indexing, so this deprecation brings us closer to the ArrayAPI.

It seems 41 commits is a bit excessive, we could squash-merge it to 1 when merging, or is there a reason to preserve some of the history here?

I think we should try to merge this early in the 1.24 version cycle, to see how much noise it creates downstream.

@Micky774
Copy link
Contributor Author

Micky774 commented Jul 12, 2022

It seems 41 commits is a bit excessive, we could squash-merge it to 1 when merging, or is there a reason to preserve some of the history here?

squash-merge is fine with me

@nschloe
Copy link
Contributor

nschloe commented Jul 13, 2022

Squash-merge does not sound good to me. I did almost all of the hard work here, and I would not like to see @Micky774 taking all of the credit for it.

Edit: I would like to challenge @Micky774's wording "Co-authored by @nschloe". This PR is not co-authored by me. It is authored by me.

@Micky774
Copy link
Contributor Author

Micky774 commented Jul 13, 2022

See relevant comment

@nschloe I'm happy to close my PR. The reason I opened my PR so quickly after asking was because I saw that there was no activity on this PR for ~7 months already, and because I wouldn't mind closing it if you were interested in continuing this one.

I just want to make it clear that this wasn't in interest of claiming credit for your work -- in the main body of my PR I laid out explicitly that the significant contributions were yours and that mine were minor reconciliations. Sorry if this caused you to worry that the work you did was going to be undermined. Again, it really is good work and I apologize that my actions caused an unnecessary stir.

@Micky774 Micky774 closed this Jul 13, 2022
@Micky774 Micky774 deleted the scalar_conversion branch May 4, 2023 19:14
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.

deprecate scalar conversions for rank>0 arrays
3 participants