Skip to content

ENH: Modify np.logspace so that the base argument broadcasts against start and stop#23275

Merged
seberg merged 1 commit intonumpy:mainfrom
roytsmart:bugfix/logspace-base-broadcast
Apr 12, 2023
Merged

ENH: Modify np.logspace so that the base argument broadcasts against start and stop#23275
seberg merged 1 commit intonumpy:mainfrom
roytsmart:bugfix/logspace-base-broadcast

Conversation

@roytsmart
Copy link
Contributor

@roytsmart roytsmart commented Feb 25, 2023

May close #23274

@roytsmart roytsmart force-pushed the bugfix/logspace-base-broadcast branch 3 times, most recently from c1aa7c7 to 287c2e5 Compare February 25, 2023 03:57
@roytsmart
Copy link
Contributor Author

@seberg, would you mind bringing this to the attention of the appropriate reviewer?

Copy link
Contributor

@mhvk mhvk left a comment

Choose a reason for hiding this comment

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

I guess not something people have thought of, but really no reason not to make it work. Would need to update the docstring (at least the axis part). Perhaps also give an example?

@mhvk
Copy link
Contributor

mhvk commented Mar 2, 2023

p.s. I don't think I'd count this as a bug, but more like an enhancement.

@roytsmart
Copy link
Contributor Author

Ok sounds good. I thought it was a bug since the documentation said that base is array like

@roytsmart roytsmart force-pushed the bugfix/logspace-base-broadcast branch 2 times, most recently from 4e6b8f5 to f90e798 Compare March 3, 2023 08:51
@roytsmart roytsmart changed the title BUG: Modify numpy.logspace so that the base argument broadcasts correctly against start and stop ENH: Modify numpy.logspace so that the base argument broadcasts correctly against start and stop Mar 3, 2023
@seberg
Copy link
Member

seberg commented Mar 6, 2023

Do you have a reasoning for where to put the base dimensions and how it would mix if start/stop are not 0-D (which seems possible)?

@roytsmart
Copy link
Contributor Author

@seberg, the way I imagined it was that base, start, and stop all had to be broadcastable against one another, no mixing allowed.

@seberg
Copy link
Member

seberg commented Mar 6, 2023

Sorry more coffee was needed, it is very obvious that broadcasting is fine. And I guess the old behavior was useless enough that it should be OK to change unceremoniously (i.e. it was mostly useless).
Do you want to add a release note and maybe a .. versionchanged:: directive to the base attribute?

@roytsmart
Copy link
Contributor Author

Ah yes, more coffee is almost always needed! I'll add those changes straight away.

@roytsmart roytsmart force-pushed the bugfix/logspace-base-broadcast branch from f90e798 to 98d3beb Compare March 6, 2023 08:52
@seberg
Copy link
Member

seberg commented Mar 6, 2023

Quick :). Changes look good, thanks. But one more ask: I am suspecting you also want (and we should) add base to the dispatcher.

@roytsmart roytsmart force-pushed the bugfix/logspace-base-broadcast branch from 98d3beb to 8c6d312 Compare March 6, 2023 09:02
@roytsmart
Copy link
Contributor Author

Ah yes, you're absolutely right. I did not know that we needed to do that, good catch!

@roytsmart roytsmart force-pushed the bugfix/logspace-base-broadcast branch from 8c6d312 to 3b89f52 Compare March 6, 2023 09:44
Copy link
Contributor

@mhvk mhvk left a comment

Choose a reason for hiding this comment

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

Looks great!

@seberg seberg changed the title ENH: Modify numpy.logspace so that the base argument broadcasts correctly against start and stop ENH: Modify np.logspace so that the base argument broadcasts against start and stop Apr 12, 2023
@seberg seberg merged commit a525da8 into numpy:main Apr 12, 2023
@seberg
Copy link
Member

seberg commented Apr 12, 2023

Thanks @byrdie.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Development

Successfully merging this pull request may close these issues.

BUG: numpy.logspace sometimes fails with a broadcasting error if base is array-like.

3 participants