Skip to content
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

Adding casting argument to astype (and other casting functions/methods) #463

Closed
jakirkham opened this issue Jul 7, 2022 · 4 comments
Closed
Labels
API extension Adds new functions or objects to the API. topic: Type Promotion Type promotion.

Comments

@jakirkham
Copy link
Member

Based on recent discussion, current astype behaves as if casting is set "unsafe". It may be worthwhile to add this argument as default "unsafe" while also allowing it to be configured. In particular "safe" may be of interest to allow users to catch any unexpected or unintended castings in library code. This casting flag may be useful in other places where castings occur.

@seberg
Copy link
Contributor

seberg commented Jul 7, 2022

For astype it seems good to me to allow it in the sense of "unsafe" casting.

However, somehow, complex -> int/float does seem less well defined then float -> int. Python has it this way, since int(1.5) is OK, but int(1j) or float(1j) is not.

So when it comes to arr.astype() I am not yet certain about removing the warning from the NumPy main namespace. But with a warning astype(complex_arr, real) seems not super useful from a library perspective (you will need to use arr.real or real(arr) to avoid the warning).

Surprisingly, am not even certain that a complex -> real needs to be defined at all (as a proper cast), are there use-cases where you need it to work but calling real() or arr.real is not clearer?

@jakirkham
Copy link
Member Author

Interesting thanks for the context.

No strong feelings about the warning in NumPy.

Main interest here is ensuring library authors don't run into subtle bugs or at least have easy ways of avoiding them. That said, maybe it is worth thinking about expectations of safe casting.

Yeah the use case for complex -> real is what I was wondering as well in this comment ( #445 (comment) ). Also not clear on why it is needed.

@leofang
Copy link
Contributor

leofang commented Jul 8, 2022

Yeah the use case for complex -> real is what I was wondering as well in this comment ( #445 (comment) ). Also not clear on why it is needed.

Trying to look at this from a different angle, I hope. Based on #427 (real) and #446 (conj) I think we are leaning toward a more restrictive API design as opposed to permissive. I seem to remember the permissive choice was favored during one of the meetings, but I can't recall it was for which library (and I also forgot which side I picked, I'd say restrictive but I am not certain anymore after a long week...😅) That's probably why I let go in #445 (astype). But, for the sake of consistency it'd be nicer if astype does not allow complex -> real?

cc: @kgryte

@rgommers rgommers added the topic: Type Promotion Type promotion. label Oct 7, 2022
@rgommers rgommers added the API extension Adds new functions or objects to the API. label Nov 30, 2022
@kgryte
Copy link
Contributor

kgryte commented Jun 29, 2023

In the specification, we opted to be restrictive in disallowing complex -> real. As such, I will go ahead and close this issue.

@kgryte kgryte closed this as completed Jun 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API extension Adds new functions or objects to the API. topic: Type Promotion Type promotion.
Projects
None yet
Development

No branches or pull requests

5 participants