Skip to content

expand_dims() signature change (and slight doc fix) #331

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

Merged
merged 2 commits into from
Nov 11, 2021

Conversation

honno
Copy link
Member

@honno honno commented Nov 10, 2021

In #167 @rgommers gave us the current expand_dims() signature:

expand_dims(x, /, *, axis)

I believe it makes sense to remove the keyword-only requirement of axis, given it has no default value:

expand_dims(x, /, axis)

This keeps in line with the related function squeeze(). Notably all other manipulation functions that have arguments after * have defaults. Personally I've never seen a keyword-only argument that has no default, and I'm wondering if this behaviour could confuse developers and users.

Copy link
Contributor

@kgryte kgryte left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks, @honno!

@kgryte kgryte added this to the v2021 milestone Nov 11, 2021
@kgryte kgryte added API change Changes to existing functions or objects in the API. Maintenance Bug fix, typo fix, or general maintenance. labels Nov 11, 2021
@kgryte kgryte merged commit 4c041ea into data-apis:main Nov 11, 2021
@honno honno deleted the expand=dims branch November 1, 2022 10:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API change Changes to existing functions or objects in the API. Maintenance Bug fix, typo fix, or general maintenance.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants