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

Add bounds check for length of passed indices and values #18

Closed
tlnagy opened this issue Sep 12, 2018 · 4 comments
Closed

Add bounds check for length of passed indices and values #18

tlnagy opened this issue Sep 12, 2018 · 4 comments

Comments

@tlnagy
Copy link

tlnagy commented Sep 12, 2018

This seems like a pretty massive bug unless I'm totally reading this wrong I was reading this wrong, but this check should still occur.

julia> c = IndirectArray([1,1,1,2,2,2], ["A", "A", "A", "B", "B", "B"])
6-element IndirectArray{String,1,Array{Int64,1},Array{String,1}}:
 "A"
 "A"
 "A"
 "A"
 "A"
 "A"

julia> versioninfo()
Julia Version 1.0.0
Commit 5d4eaca0c9 (2018-08-08 20:58 UTC)
Platform Info:
  OS: Linux (x86_64-pc-linux-gnu)
  CPU: AMD Ryzen 7 2700X Eight-Core Processor
  WORD_SIZE: 64
  LIBM: libopenlibm
  LLVM: libLLVM-6.0.0 (ORCJIT, znver1)

IndirectArrays v0.5.0

@tlnagy
Copy link
Author

tlnagy commented Sep 12, 2018

julia> dump(c)
IndirectArray{String,1,Array{Int64,1},Array{String,1}}
  index: Array{Int64}((6,)) [1, 1, 1, 2, 2, 2]
  values: Array{String}((6,))
    1: String "A"
    2: String "A"
    3: String "A"
    4: String "B"
    5: String "B"
    6: String "B"

So it looks like it is working, but it needs a bounds check or something because it requires that unique(index) == length(values) but this isn't enforced.

@tlnagy tlnagy changed the title Custom Indices ignored when passed to constructor Add bounds check for length of passed indices and values Sep 12, 2018
@nalimilan
Copy link
Contributor

Strictly speaking what it required is unique(index) ⊆ 1:length(values). And AFAICT that's enforced:

julia> IndirectArray([1,10], ["a"]);
ERROR: BoundsError: attempt to access 1-element Array{String,1} at index [[1, 10]]

Providing some duplicated and some unused levels as in your example isn't very useful, but I'm not sure it should be forbidden.

@timholy
Copy link
Member

timholy commented Sep 12, 2018

Right, a good example is let's say you want to provide a gray color scale for an indexed image, but color saturated pixels red. I might get a constant stream of images from my camera, some of which might have saturated pixels and others not; I don't want to have to change my color map dependent on whether pixels actually "hit" each value or not.

@tlnagy
Copy link
Author

tlnagy commented Sep 12, 2018

@timholy your explanation makes a lot of sense. I was suspecting that something like that was the reasoning. Thanks!

@tlnagy tlnagy closed this as completed Sep 12, 2018
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

3 participants