Added save compatible version of filenames, arrays and images function

[siddharthlal25]

One of the most important functions of this package!
Related issue: #28

[codecov]

Codecov Report

Merging #30 into master will decrease coverage by 0.06%.
The diff coverage is 85.18%.

@@            Coverage Diff             @@
##           master      #30      +/-   ##
==========================================
- Coverage   91.73%   91.66%   -0.07%     
==========================================
  Files           4        4              
  Lines         121      168      +47     
==========================================
+ Hits          111      154      +43     
- Misses         10       14       +4     

Impacted Files	Coverage Δ
src/fits.jl	93.75% <ø> (ø)
src/collection.jl	85.00% <85.18%> (+9.24%)	⬆️

---

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 5135942...a7dd75f. Read the comment docs.

[mileslucas]

My ideal interface would have three versions of this, images, arrays, filenames, to match the three functions we currently have that support iteration. This gives a consistent interface across the API- I can go from

for img in images(coll)
	# ...
end

to this:

images(coll) do img
    # ...
end

and get the same functionality, plus the saving if I want it.

[mileslucas]

Okay, skimming over the three versions, I think we should have a little more granularity in the functionality, like I mention with splitting the make_file function.

In addition, the current mechanics are kind of hard to understand- if path is not nothing we will save files. Where will you save files? Is this path supposed to be the load path, too? What is being saved? Can I do a dry-run without actually saving?

Some of these just require some documentation, but I really think we need to re-evaluate the functionality of the path argument in its current state and either change the name, add more documentation, or split its functionality into two parts (eg path is just the save path, which defaults to same folder as the file and the save mechanics can be manually enabled/disabled with save).

I left some comments about using map style instead of the for-loop, in addition I want you to think about how we can use the functions that already exist to greatly simplify our API. Consider what the current images function can do- iterate over the dataframe with some options and provide each ImageHDU. What are you doing here? You're iterating over the dataframe with some options, providing the ImageHDU, applying a user-provided function, then adding some more new options. This sounds like you could probably write this function to simply take

function images(f, df; options_related_to-saving..., kwargs...)
    processed_images = map(images(df; kwargs...)) do img
        # do your thing
        return processed_img
    end
    return processed_images
end

Hopefully that makes sense! This will be invaluable if you ever add an extra keyword argument to the looping-only functions and don't want to have to rewrite all of these as well! The basic idea is that all these functions do is the same iteration but with some fun file-saving mechanics. Everything else should work the same.

[mileslucas]

We will definitely want to show some examples with this, I'm not sure whether it makes more sense to copy-paste three nearly identical examples in the docstrings or whether to relagate that to the online documentation. Perhaps both?

Another way to get around this is to put all of the filename-changing stuff into the docstring for make_file and then show the CCDReduction.make_file docstring in the docs and reference it. This removes the repitition and keeps all the relevant info in docstrings, but at the cost of having to search around a little more to get the information you need.

[mileslucas]

some docs comments- should propagate to all three versions

[mileslucas]

Suggested change

	Iterative version of `arrays`, returns data loaded by `FITSIO` using `CCDReduction.getdata` into an `Array`.
	It utilizes the `path` and `hdu` from the `collection`.
	Iterates over `collection` using each `path` and `hdu` to load data using [`CCDReduction.getdata`](@ref) into an `Array`.

[siddharthlal25]

I have used:

[`CCDReduction.getdata`](@ref)
[`CCDReduction.write_fits`](@ref)

This fails the building of docs, some warning that shows up:

┌ Warning: no doc found for reference '[`CCDReduction.write_fits`](@ref)' in src/api.md.
└ @ Documenter.CrossReferences ~/.julia/packages/Documenter/PLD7m/src/CrossReferences.jl:160
┌ Warning: no doc found for reference '[`CCDReduction.write_fits`](@ref)' in src/api.md.
└ @ Documenter.CrossReferences ~/.julia/packages/Documenter/PLD7m/src/CrossReferences.jl:160
┌ Warning: no doc found for reference '[`CCDReduction.write_fits`](@ref)' in src/api.md.
└ @ Documenter.CrossReferences ~/.julia/packages/Documenter/PLD7m/src/CrossReferences.jl:160

If I don't cross-reference the documentation, the doc gets build completely. Is it a bug with Documentation.jl?