Skip to content

Conversation

@BvB93
Copy link
Contributor

@BvB93 BvB93 commented Sep 28, 2021

Xref #143 and numpy/numpy#19969.

The spec currently claims that the k parameter of eye should accept either None or an integer, however not a single mention is made of None in the actual parameter-description. In fact, support for k=None seems to be absent from the limes of np.eye as well.

Based on this it seems the inclusion of the k: Optional[int] = ... annotation may have been accidental; hence the Optional encapsulation is removed in this PR. If this was a deliberate choice I'd instead propose to update the docs with a more explicit mention of None and its expected behavior.

@asmeurer
Copy link
Member

This looks good to me, but others should confirm that this is indeed what was intended.

@rgommers rgommers added the topic: Static Typing Static typing. label Sep 29, 2021
Copy link
Member

@rgommers rgommers left a comment

Choose a reason for hiding this comment

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

Thanks @BvB93. This seems like a simple copy-paste mistake, or assuming "optional" means "don't have to specify" rather than "| None". This issue wasn't discussed in the original PR (gh-31). In it goes.

@rgommers rgommers merged commit be6d7c5 into data-apis:main Sep 29, 2021
@BvB93 BvB93 deleted the eye branch September 29, 2021 20:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

topic: Static Typing Static typing.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants