beta-effect / juxta.jl Goto Github PK

I'm probably just misunderstanding but I thought when you select part of a JuxtArray, either through physical slicing or index-based slicing, that you would get back a new JuxtArray containing just the selected portion, which shouldn't modify the existing JuxtArray.

But it seems that these functions actually modify the existing JuxtArray and overwrite ja.array? Don't you lose data this way?

Was just wondering about a few things:

1. Why make the struct mutable? Looks like the array contents may change but the struct itself won't so making it immutable might make more sense? You can append attributes as well, just not change what attribs is. Also could help the compiler make certain optimizations as it knows the struct properties won't change.
2. I'm wondering if it would be better to use Symbols instead of Strings as the key. Not sure how helpful it would be for a labelled array but other packages like DataFrames.jl seem to use symbols. Might be a good read: https://stackoverflow.com/questions/23480722/what-is-a-symbol-in-julia
3. I used to use @assert but apparently in C/C++, Python, and Julia they seem more for checking developer mistakes and may actually be disabled by certain optimization levels. So might be better to throw exceptions or something else.

Also, I might strongly type the struct to get more flexible JuxtArray (i.e. keys can be symbols or strings, types sort themselves out, indices don't need to be abstract ranges, etc.):

struct JuxtArray{A, D, C, AT, I}
     array::A
     dims::D
     coords::C
     attribs::AT
     indices::I
end

or you can be a bit more specific if you want to:

struct JuxtArray{A<:AbstractArray, K, C, I}
     array::A
     dims::Vector{K}
     coords::Dict{K, C} 
     attribs::Dict
     indices::Dict{K, I} 
end

Could probably dispatch on method if it was a type or via the Val type although I think the latter is discouraged.

If you include code examples in the README with

```julia <code>```

then you also get syntax highlighting which might make them a bit more readable.

Looks like you already have great docstrings. If you're interested in automatically turning it into documentation, check out https://github.com/JuliaDocs/Documenter.jl

should be Juxta.jl.

can be rewritten as something like

push_dim!(ja, k, v::OrdinalRange) = push!(ja.indices, String(k) => v)
push_dim!(ja, k, v::Integer)      = push!(ja.indices, String(k) => v:v)

...

String(k) in ja.dims && push_dim!(ja, k, v)

which should be easier to extend to other types of v. It also seems to be the more Julian way of doing things.

If ja.dims was of type Vector{Symbol} you might be able to just use k instead of String(k).