tickets/DM-2931: truncation in Wcs persistence #30
Conversation
N.b. _mayBePersistable captures the basic projection-independent checks
This is necessary because the base Wcs class can no longer be persisted.
N.b. This was always the desired functionality; this change set makes it work
| auto archiveWcs = archive.get<Wcs>(wcsId); | ||
| if (archiveWcs) { | ||
| _wcs = archiveWcs; | ||
| } |
There was a problem hiding this comment.
It looks like the warning message below this block is incorrect: we should say we're just using the Wcs we got from the header.
By the way, I just refreshed my memory on when that warning is supposed to appear: it's when we couldn't load the object because it's defined in an extension package that's not setup. If the WCS wasn't saved in the tables at all, we'll just get a null pointer back, and the block above is in play. If the WCS was saved but it's been corrupted, we'll get an exception that propagates all the way up.
There was a problem hiding this comment.
I fixed the comment
I also note that we could check for wcsId == 0, but doing that would break the symmetry with e.g. Psf handing and I didn't want to fix all of them on this ticket.
|
Is it possible to write test cases for this code? |
| // The current table persistence only works for TAN and TAN-SIP projections | ||
| return false; | ||
| } | ||
|
|
There was a problem hiding this comment.
I don't think we need to call _mayBePersistable() at all here, since we're going to return false regardless.
There was a problem hiding this comment.
I wrote it that way as a template for derived classes. It doesn't cost enough to matter.
Also added an info-level message when we use the FITS header not the table N.b. this code would be rewritten to check if wcsId == 0, but that would break the symmetry with the way that Jim reads the other entries such as the Psf
No description provided.