-
Notifications
You must be signed in to change notification settings - Fork 12
Experimental ASDF file read support #631
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
Open
embray
wants to merge
29
commits into
astrorama:develop
Choose a base branch
from
embray:asdf
base: develop
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Conversation
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
…o test Nothing that does anything here yet, just bare minimum to test libasdf detection and compile something with it.
Extend the existing (barely used) FitsReader class to implement ImageFileReader Extend FitsImageSource to also allow selecting FITS HDUs by extname (should probably support extver as well, but this is less common)
This will be useful for creating a DetectionImage from some non-FITS file type; mostly as a placeholder until reading a WCS from ASDF is implemented.
...and prepare for the possibility of non-FITS images.
HDUs (even skipping over non-image HDUs) Consequently make FitsReader::get(int) more sane: it returns the N-th image HDU in the file (0-indexed) rather than FITSish 1-indexing. This enables more consistent use of ImageFileReader::get regardless of the underlying file type.
Ndarray objects out of it. There is a bit of a foot-gun here that AsdfFile has to be wrapped in shared_ptr when using this. I may try to get rid of that limitation later.
…e_2d This will be used to implement AsdfImageSource::getImageTile
classes Drop use of enable_shared_from_this on AsdfFile which was misguided. Should clarify that the lifetime of an `AsdfFile::Ndarray` is concurrent with the `AsdfFile` itself (which is kept alive so long as we have a pointer to its `FileAccessor`)
by the ImageFileReader.get() and iterator interfaces Make this work again for FITS--for ASDF we need more work on the libasdf side.
latest updates to libasdf's development API
(I'm not even exactly sure what this is for yet, but it nicely demonstrates how relatively easy it is now to abstract out the file type)
Needed to add setLayer in the ImageSource base class for convenience; by default this can just be a no-op where it's not applicable.
FITS tests--just their mirror images roughly Header metadata isn't copied over yet though.
tests for FitsImageSource_test
ImageFileReader::readImage on the base class--better name and more general Also fixes handling of image_type in FitsReader::get
This caught (and now fixes) some bugs in how the ImageTile::ImageType was being passed through to AsdfImageSource::getImageTile
theory) which will make it easier to avoid having to write different special cases for FITS and ASDF files specifically (even though they're the only applicable cases for now) For the ASDF case specifically support extracting a FitsWCS. Right now just provides a wrapper around the relevant libasdf GWCS extension types--still need to then plug that into WCSLIB and add tests
supported) and initializing the wcsprm, etc. Adds some tests based on the once for FITS but as noted it's a different WCS with different results. I tested the expected against wcslib and GWCS as well.
this will be useful when matching WCS's to images
path for detection images and reference images this just sets up the wiring--still need to implement parsing and interpretation of the WCS path (and figurout where to document the format)
kinda hacky but gets the job done for now; later may change this when the libasdf gwcs module gets moved out to a separate library/plugin
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.
Follow-up to the discussion I opened at #630 to maybe put a little more concreteness behind it.
This primarily does two things:
If it makes it easier I can also split this up into separate PRs, e.g. one for the general refactoring and one with the ASDF-specific stuff.
Writing ASDF files is not supported yet (as it's not yet supported in libasdf, though will be in a later version). So generalization of file output is not part of this.
Currently libasdf has a source-only release but we are also working on providing a conda package for it in conda-forge.