Repository navigation
Conversation
…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
libasdf-gwcs is now a separate package, which was not the case when I last worked on this, so the build system needs to be updated to check for libasdf-gwcs as well, including AST backend support. This also makes a few fixes to existing code that was affected by libasdf API changes so that it's building and the tests passing again before I start work on GWCS suppport.
The getFitsHeaders method is now moved into its own mixin interface class FitsWcsSerializable. For now this is only applicable to the WCS coordinate system, though there are schemes, to be added later, for serializing GWCS to FITS as well. Also adds previously missing test coverage for writing a FITS image with round-tripping of the WCS headers (as used for check images), and the counter example (simulating ASDF+GWCS) where the WCS cannot be output (yet) to the check image.
The ImageSource base class gains ImageSource::getCoordinateSystem for returning any CoordinateSystem (WCS or some future GWCS) as determined appropriate by the ImageSource. This feels like a better level of abstraction, as different ImageSources may have one or more possible ways to represent a coordinate system, and contain the most knowledge about how to select for it. This also fully decouples WCS.h from specific ImageSource implementations (FitsImageSource or the new AsdfImageSource), keeping the WCS implementation more file format agnostic (modulo the parts that are inherent to FITS data structures). Also deleted the generic WCS(ImageSource&) constructor which was actually a bug: Both DetectionImageConfig.cpp and MeasurementImageConfig.cpp had a bug here were a specific ImageSource instance quietly degraded its type to the ImageSource base class, resulting in quietly producing a bogus identity WCS when passed to `WCS(*image_source)`.
This generalizes the notion of "path to the WCS" across supported image formats. This leads to a little bit of awkwardness like supportsWcsPath, as this is not currently meaningful in FITS. However, it's designed with the possibility in mind that it could be meaningful in FITS, e.g. when adding support for ASDF-in-FITS--this is used I think on some JWST data products that embed a full GWCS as ASDF/YAML embedded in a FITS file, but also ships an approximated FITS WCS. Generalizing it also allows deleting several ASDF-specific code paths. Trying to provide a wcs_path to a file that doesn't know what to do with it results in a meaningful warning message in the log, but is otherwise ignored. Note: setWcsPath is implemented on the ImageReader as opposed to ImageSource, as the latter typically represents a single image in a file (like a single FITS HDU) whereas setting the WCS path is more of a file-level operation for now (makes sense when iterating over an ImageFileReader). That said, I'm not convinced myself that that's really the correct reasoning, and might consider revisiting this later.
…gwcs This removes the old code I had for hoisting a WCSLIB WCS from an ASDF file in the narrow case of a lone fitswcs_imaging--might be worth revisiting later so that code is still in the history, but now most of it isn't needed. The old ASDF->WCS tests are rewritten to use the GWCS instead and agrees well with the previous results evaluated through WCSLIB. Still work to be done on figuring out how to handle inverses, and on thread safety.
The only libasdf-gwcs evaluation backend currently supported, AST, holds an AST FrameSet underlying the evaluation context (asdf_gwcs_eval_t). This is pinned to the thread on which it was created, and cannot be shared safely between threads. This uses the newly introduced `asdf_gwcs_eval_copy` routine, which safely copies this context even between threads. Although this may not be necessary for all future hypothetical backends, it's probably best to do in general. libasdf-gwcs hasn't committed yet to whether or not an `asdf_gwcs_eval_t` is always considered immutable or safe to use across threads, so best to copy it before each use which is relatively cheap.
The per-implementation .test() method works on an istream directly so if the stream is not readable the test simply fails; SE++ reports that the file type is not recognized. In fact it should check if the file exists and report more sensible error messages in this case. Adds some tests for ImageFileReader's interface.
This branch has not been deployed
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
1 participant
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.