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

Make isassigned(A, idx...) the generic way to test if A[idx...] is valid #18843

Closed
cnaak opened this issue Oct 8, 2016 · 13 comments
Closed

Make isassigned(A, idx...) the generic way to test if A[idx...] is valid #18843

cnaak opened this issue Oct 8, 2016 · 13 comments

Comments

@cnaak
Copy link

cnaak commented Oct 8, 2016

julia> text = "Light in 日本語 is 光"
"Light in 日本語 is 光"

julia> i = 1;

julia> while i <= endof(text)
           @printf "%02d --> %2s --> 0x%-4s\n" i text[i] hex(Int32(text[i]))
           i = nextind(text, i)
       end
01 -->  L --> 0x4c  
02 -->  i --> 0x69  
03 -->  g --> 0x67  
04 -->  h --> 0x68  
05 -->  t --> 0x74  
06 -->    --> 0x20  
07 -->  i --> 0x69  
08 -->  n --> 0x6e  
09 -->    --> 0x20  
10 -->--> 0x65e5
13 -->--> 0x672c
16 -->--> 0x8a9e
19 -->    --> 0x20  
20 -->  i --> 0x69  
21 -->  s --> 0x73  
22 -->    --> 0x20  
23 -->--> 0x5149

julia> isdefined(text, 23), text[23]
(false,'')

So text[23] returns a character, while isdefined(text, 23) returns false.

(On a side note, light in Japanese (and Chinese) looks like a "JL" — of julia — under the Sun: 光).

I'm running julia-0.5.0, commit 3c9d753 (2016-09-19 18:14 UTC)

@yuyichao
Copy link
Contributor

yuyichao commented Oct 8, 2016

isdefined should not be used to check anything other than object fields.

@yuyichao yuyichao changed the title isdefined() seems not to work for Unicode Strings isassigned() does not to work for String Oct 8, 2016
@yuyichao
Copy link
Contributor

yuyichao commented Oct 8, 2016

And it's unclear if we want to support isassigned on string either. So far isassigned is only defined for array like types.

@cnaak
Copy link
Author

cnaak commented Oct 8, 2016

Comming from Python, my expectation is that isdefined() should work for any iterable... and the documentation [http://docs.julialang.org/en/release-0.5/stdlib/base/#Base.isdefined] has:

isdefined(object, index::Int)
isdefined(a::Array, index::Int)

Can't a String be taken either as an object or an array (of Chars)?

@yuyichao
Copy link
Contributor

yuyichao commented Oct 8, 2016

my expectation is that isdefined() should work for any iterable

No it will not. That's not what isdefined is. isdefined should only be used to check field (edit: including module field/members) and it should never be used on arrays. (That it supports Array at all is an unfortunate historical artifact and is deprecated already)

the documentation has:
isdefined(object, index::Int)
isdefined(a::Array, index::Int)

Please read the document text itself to see what the first signature actually means.

@cnaak
Copy link
Author

cnaak commented Oct 8, 2016

I've read it again, and it's not that clear (at least to a julia newbie like me), since I don't know yet what a "composite object" is and whether or not a String can be deemed as an "Array" (based on your answer now it's clear that it can't).

For closure, is there any analog method that will work for Strings, that will take care of the index "gaps" left by wider characters as illustrated in the while loop in the question?

@yuyichao
Copy link
Contributor

yuyichao commented Oct 8, 2016

I've read it again, and it's not that clear (at least to a julia newbie like me), since I don't know yet what a "composite object" is

You can find this in the doc

is and whether or not a String can be deemed as an "Array" (based on your answer now it's clear that it can't).

julia> ""::Array
ERROR: TypeError: typeassert: expected Array{T,N}, got String

And this confusion is also why isdefined should never be used on arrays or mentions arrays at all. It's gone on master http://docs.julialang.org/en/latest/stdlib/base/#Base.isdefined

For closure, is there any analog method that will work for Strings, that will take care of the index "gaps" left by wider characters as illustrated in the while loop in the question?

No, but the closest we have right now is isassigned. It's currently working as a testing function for getindex on an almost superset (except tuples) of isdefined but is still mainly limited to AbstractArrays. I do think it can be made to do the job similar to isdefined but for getindex instead of getfield and that's why I edited the title.

@cnaak
Copy link
Author

cnaak commented Oct 8, 2016

Thanks for the precision!

Oh, I recall now of a previous test I made, which led me to believe that isdefined(text, 23) in the example should work:

julia> text = "Light in 日本語 is 光"
"Light in 日本語 is 光"

julia> isdefined(text, 1)
true

So isdefined(text, 1) returning true but isdefined(text, n) for any n>=2 returning false seems to be inconsistent at least. Based on your explanation, they all should fail, based on the wrong type: String.

@yuyichao
Copy link
Contributor

yuyichao commented Oct 8, 2016

Based on your explanation, they all should fail, based on the wrong type: String.

No, isdefined has a single defined meaning for all object, i.e. if the given field of an object is defined, i.e. it's what hasattr in python does. By isdefined(text, 1) you are asking if getfield(text, 1) is valid and it correctly tells you that the object has a single field that is defined.

See comments in #18086 for why isassigned should be used instead of isdefined for this and #18362 for the PR that removes this confusion on master.

@yuyichao yuyichao changed the title isassigned() does not to work for String Make isassigned(A, idx...) the generic way to test if A[idx...] is valid Oct 8, 2016
@nalimilan
Copy link
Member

I think what we need here (and in many other places) is a See also docstring section listing isassigned so that people easily spot the difference.

@yuyichao
Copy link
Contributor

yuyichao commented Oct 8, 2016

Well, on master there's no similarity between the two anymore and on 0.5 it does mention isassigned.

@cnaak
Copy link
Author

cnaak commented Oct 8, 2016

@yuyichao:

By isdefined(text, 1) you are asking if getfield(text, 1) is valid and it correctly tells you that the object has a single field that is defined.

What is the "object" in @yuyichao 's sentence? (i) text, or (ii) text[1], or (iii) text[1:1], or (iv) something else?

If (i), then isdefined(text, 1) is not doing anything with the index 1, and thus behaving inconsistently for indices >1; if (ii), then the object is another String, just like text[n] for n>=2 and thus behaving inconsistently as well; if (iii), then the object is a Char, and the same inconsistency argument of (ii) can be developed; if (iv) I just can't think of anything else that would make sense...

Also, from the quote, what is the "single field that is defined" specified (or obtainable) by text and the index 1 that is not so by text and an index other than 1?

@yuyichao
Copy link
Contributor

yuyichao commented Oct 8, 2016

What is the "object" in @yuyichao 's sentence? (i) text, or (ii) text[1], or (iii) text[1:1], or (iv) something else?

text

If (i), then isdefined(text, 1) is not doing anything with the index 1,

It never does and shouldn't do that.

and thus behaving inconsistently for indices >1

It's consistent because it does not do anything about indexing for any index values.

Also, from the quote, what is the "single field that is defined" specified (or obtainable) by text and the index 1 that is not so by text and an index other than 1?

dump(text). I've already mentioned that

By isdefined(text, 1) you are asking if getfield(text, 1) is valid

And you can find out what getfield does in the doc too http://docs.julialang.org/en/latest/stdlib/base/?highlight=getfield#Base.getfield

@oscardssmith
Copy link
Member

We aren't going to do this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants