Skip to content

Clarify that the results of division operations on integer array data types resulting in floating-point outputs is implementation-defined #362

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 5 commits into from
Jan 17, 2022

Conversation

honno
Copy link
Member

@honno honno commented Jan 7, 2022

Resolve #361 by noting any instance of int as out-of-scope.

@asmeurer
Copy link
Member

asmeurer commented Jan 8, 2022

This is what I wanted to do in #221, but it was rejected. I still think it's a good idea.

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.

This should say why, otherwise it leaves the reader guessing. The pow note says:

If self has an integer data type and other has a floating-point data type, behavior is implementation-dependent (type promotion between data type “kinds” (integer versus floating-point) is unspecified).

I'd like to be a little more strict even, because there really are only two outcomes possible: a floating point array with the expected division result, or an exception. Doing Python 2 style division should be forbidden. So how about saying that explicitly in this note?

@honno honno requested review from rgommers and asmeurer January 10, 2022 13:31
@rgommers rgommers added the Maintenance Bug fix, typo fix, or general maintenance. label Jan 10, 2022
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.

This LGTM. Let's see if everyone else is happy too before merging.

Copy link
Contributor

@leofang leofang left a comment

Choose a reason for hiding this comment

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

Leaving two comments below. I guess I find the fix suggested by @rgommers a bit confusing:

I'd like to be a little more strict even, because there really are only two outcomes possible: a floating point array with the expected division result, or an exception. Doing Python 2 style division should be forbidden. So how about saying that explicitly in this note?

If the spec requires the returned array to be of floating-point type if not raising an error, then it is not fully "implementation defined". We should either

  1. leave it completely implementation defined, or
  2. remove the ambiguity by saying the implementation must pick a floating point type (but which to pick is up to the implementation)

@rgommers
Copy link
Member

If the spec requires the returned array to be of floating-point type if not raising an error, then it is not fully "implementation defined".
...
remove the ambiguity by saying the implementation must pick a floating point type (but which to pick is up to the implementation)

+1 for this. Isn't that the same as what I said?

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.

@kgryte kgryte changed the title Make int inputs for divide functions out-of-scope Clarify that the results of division operations on integer array data types resulting in floating-point outputs is implementation-defined Jan 17, 2022
@kgryte kgryte merged commit 35671a4 into data-apis:main Jan 17, 2022
@kgryte
Copy link
Contributor

kgryte commented Jan 17, 2022

Thanks, @honno!

@honno honno deleted the div-ints branch November 1, 2022 10:51
@kgryte kgryte added this to the v2021 milestone Dec 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Maintenance Bug fix, typo fix, or general maintenance.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Divide functions have ambiguous specification for integer inputs
5 participants