Conversation
Also add doc to readme
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #16 +/- ##
==========================================
+ Coverage 85.91% 86.46% +0.54%
==========================================
Files 16 18 +2
Lines 3230 3376 +146
==========================================
+ Hits 2775 2919 +144
- Misses 455 457 +2 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Member
|
Nice, I'm excited to try this, thanks Chris! I'll go ahead and get this, JuliaAstro/AstroImages.jl#113, and JuliaAstro/AstroImages.jl#115 merged in so that we can explore integrating |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #15.
Adds a new sliced WCS wrapper
SlicedWCSTransformand an abstract typeAbstractWCSTransformthat bothSlicedWCSTransformandWCSTransformare subtypes of to support common dispatch.API:
slice_wcs(::AbstractWCSTransform, slices...) -> SlicedWCSTransformSlicedWCSTransformcontains the parent WCSTransform object and the necessary information to support translating between its own pixel space to the original parent's pixel space. Calls to e.g.pixel_to_world(swcs::SlicedWCSTransform, pixel)basically reconstruct the pixel of original parent WCSpix_parentfrom the providedpixeland callpixel_to_world(swcs.parent, pix_parent). There is some additional bookkeeping around dropped axes, e.g.swcs = slice_wcs(::WCSTransform, 1:10, 1:10, 1)drops the last axis (common for 3-D spectral cubes) so that nowpixel_to_world(swcs, [1.0, 1.0])works because the last dimension has been dropped. Similarly,swcs = slice_wcs(::WCSTransform, 1, 1, :)will drop the first two axes and produce aswcswith a pixel dimension of 1.Recursive slicing
slice_wcs(::SlicedWCSTransform, slices...)is supported.slicesmust be of equal dimension to the input WCS or less. If dimensionality of slices is lower than dimensionality of input WCS, the missing dimensions are assumed to be passed through unaltered.A method
slice_wcs(::Dict{Char, <:AbstractWCSTransform}, slices...) -> Dict{Char, SlicedWCSTransform}is defined that just callsslice_wcson each entry in the dictionary and returns a new dictionary with the same keys where all WCS have been sliced. This is so that downstream code that usesWCS_allcan just callslice_wcs(WCS_all(hdr), slices...)and simultaneously slice all the alternate WCS.Performance seems pretty good (<1 microsecond per
slice_wcs(::AbstractWCSTransform, slices...)) but I haven't done much optimization work.Also added some API functions
pixel_n_dim,world_n_dim,axis_correlation_matrixwhich simplify some of the implementation. I believe these methods respect astropy's APE14 standard but I didn't cross-reference against their implementations to check if they are exactly the same or not.@icweaver let me know how this looks and whether you need any additional features/API to support https://github.com/JuliaAstro/AstroImages.jl/pull/113/changes
If this looks good as-is, you can merge this