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

isimag: what should it do? #19947

Closed
Sacha0 opened this issue Jan 9, 2017 · 12 comments
Closed

isimag: what should it do? #19947

Sacha0 opened this issue Jan 9, 2017 · 12 comments
Labels
design Design of APIs or of the language itself domain:complex Complex numbers domain:maths Mathematical functions

Comments

@Sacha0
Copy link
Member

Sacha0 commented Jan 9, 2017

#19925 (comment) and the following discussion revealed lack of consensus regarding isimag's behavior. Clarifying isimag's behavior is this thread's purpose. Questions that prompted the original discussion: Should isimag(0) yield true or false? What about isimag(0+0im)? Thoughts? Best!

@Sacha0 Sacha0 added design Design of APIs or of the language itself domain:maths Mathematical functions labels Jan 9, 2017
@TotalVerb
Copy link
Contributor

As far as I can tell, nobody actually uses this function. It seems only to exist for consistency with isreal.

@stevengj
Copy link
Member

stevengj commented Jan 9, 2017

It's not used in any registered package as far as I can tell. I expect that it is only in Base because it was assumed to be needed for completeness, given isreal.

If we have it at all, I think the current definition isimag(z) = iszero(z.real) is the only sane one, but it might make more sense just to deprecate it.

@ararslan
Copy link
Member

ararslan commented Jan 9, 2017

Quoth the Jeff,

I'm not sure what this function is even for

I certainly sympathize with that. I would imagine that isimag = !isreal but that precise behavior is weird in practice. Deprecating it sounds like a good idea to me, unless we restrict it to complex inputs only.

@TotalVerb
Copy link
Contributor

I also agree that we should deprecate isimag(z) to iszero(real(z)).

@tkelman tkelman added the domain:complex Complex numbers label Jan 9, 2017
@stevengj
Copy link
Member

stevengj commented Jan 9, 2017

Note that iszero is not yet exported, so we would need to export it first in order to use that deprecation.

@ararslan
Copy link
Member

ararslan commented Jan 9, 2017

iszero seems nice to have exported, but if there's a good reason it isn't exported, perhaps the deprecation could just be real(z) == 0.

@stevengj
Copy link
Member

stevengj commented Jan 9, 2017

iszero was added in #17623, where I suggested that it should be exported, but that PR was doing too many other things so I chose not to export iszero just to be conservative (and to avoid arguments over its semantics). A PR to export iszero would be welcome.

But I agree that real(z) == 0 is fine for deprecating isimag(z::Complex).

@Sacha0
Copy link
Member Author

Sacha0 commented Jan 9, 2017

Note that iszero is not yet exported, so we would need to export it first in order to use that deprecation.

If isimag isn't much used, deprecating with Base.iszero(real(z)) seems reasonable? Avoids coupling deprecation of isimag to the question of exporting iszero.

@ararslan
Copy link
Member

ararslan commented Jan 9, 2017

Doing the deprecation and export together would actually work well, I would think, because if we deprecate in favor of Base.iszero but have iszero exported, that's kind of confusing. But as you said, if isimag isn't used much then likely few people would even notice the deprecation warning anyway.

@Sacha0
Copy link
Member Author

Sacha0 commented Jan 9, 2017

Tentative deprecation PR using the Base.iszero(real(z)) approach at #19949. Best!

@ararslan
Copy link
Member

ararslan commented Jan 9, 2017

And exporting PR at #19950. 🙂

@JeffBezanson
Copy link
Sponsor Member

Closed by #19949.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
design Design of APIs or of the language itself domain:complex Complex numbers domain:maths Mathematical functions
Projects
None yet
Development

No branches or pull requests

6 participants