Generalize the definition of Map and Alm

[ziotom78]

This is a WIP to address issue #24 opened by @xzackli. The definition of Map and Alm have been modified, and now AA <: AbstractArray{T, 1} has been added to the list of parameters for these two parametric types.

The PR tries to preserve compatibility with old code by adding new constructors for Map and Alm that assume AA = Array{T, 1}. As a result, I only had to do minimal changes to the test cases.

What is still missing is to decide how should functions that create Map or Alm objects behave (e.g., when readMapFromFITS returns a map loaded from disk, which kind of array should create?). At the moment I chose to use Array{T, 1} types, but comments are accepted!

Things to do:

• Redefine Map
• Redefine Alm
• Fix all function definitions
• Add test cases using SharedArray objects
• Update the documentation

[codecov]

Codecov Report

Merging #26 into master will decrease coverage by 0.03%.
The diff coverage is 52.94%.

@@            Coverage Diff             @@
##           master      #26      +/-   ##
==========================================
- Coverage   80.92%   80.89%   -0.04%     
==========================================
  Files          18       18              
  Lines         755      764       +9     
==========================================
+ Hits          611      618       +7     
- Misses        144      146       +2     

Impacted Files	Coverage Δ
src/projections.jl	65.51% <0.00%> (ø)
src/map_pixelfunc.jl	56.25% <12.50%> (ø)
src/map.jl	47.82% <29.41%> (+9.73%)	⬆️
src/conformables.jl	66.66% <66.66%> (ø)
src/polarizedmap.jl	62.50% <83.33%> (-37.50%)	⬇️
src/sphtfunc.jl	95.77% <87.50%> (ø)
src/alm.jl	95.83% <100.00%> (+0.27%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 93411ec...7052d41. Read the comment docs.

[xzackli]

I think it's very reasonable to read Array{T,1} from disk, since that is really the type that other HEALPix libraries operate on.

[xzackli]

Before this is merged, it may be helpful to copy and paste the existing tests for map and alm but substitute SharedArray, since that array type is in the standard library.

[xzackli]

Hey @ziotom78, I've tested it out a bit, could it be possible to replace a collect constructor with add a generic array constructor? i.e. replace this function,

Alm{T}(lmax, mmax, arr) where {T <: Number} =
    Alm{T, Array{T, 1}}(lmax, mmax, collect(arr))

with this one

Alm(lmax, mmax, arr::AA) where {T <: Number, AA <: AbstractArray{T,1}} =
    Alm{T, AA}(lmax, mmax, arr)

Then you don't have to repeat the array type twice if you're constructing something, and collect is unnecessary since we are no longer based on Array. For example, it'd be nice in sphtfunc, like this set of lines,

    alms = [Alm{ComplexF64}(lmax, mmax, zeros(ComplexF64, nalms)),
            Alm{ComplexF64}(lmax, mmax, zeros(ComplexF64, nalms)),
            Alm{ComplexF64}(lmax, mmax, zeros(ComplexF64, nalms))]

Since the constructor can already dispatch on the element type of the input array as well as the array type passed, this can be simplified to

    alms = [Alm(lmax, mmax, zeros(ComplexF64, nalms)),
            Alm(lmax, mmax, zeros(ComplexF64, nalms)),
            Alm(lmax, mmax, zeros(ComplexF64, nalms))]

[ziotom78]

Nice idea @xzackli, I like it! I had to modify several calls to Alm constructors, but now all tests pass. The new syntax is really nicer!

[ziotom78]

@xzackli, what do you think about the PR? If it looks good, I can merge it.

[xzackli]

This looks great, these are some exciting changes!