Uploaded image for project: 'Data Management'
  1. Data Management
  2. DM-2599

afw.Image.ExposureF('file.fits.fz[i]') returns the image in 'file.fits.fz[1]'

    Details

    • Type: Bug
    • Status: Done
    • Resolution: Done
    • Fix Version/s: None
    • Component/s: afw
    • Labels:
      None
    • Story Points:
      3
    • Sprint:
      Science Pipelines DM-S15-2, Science Pipelines DM-S15-3, Science Pipelines DM-S15-4
    • Team:
      Data Release Production

      Description

      It seems that afwImage.ExposureF ignores the extension number when this is passed on as part of the filename and uses the image in extension number 1. This is not the case with afwImage.MaskedImageF which correctly uses the input extension number passed in the same way.

      The problem has been checked on OSX Yosemite 10.10.3 with
      the is illustrated in the following code https://gist.github.com/anonymous/d10c4a79d94c1393a493

      which also requires the following image in the working directory:
      http://www.astro.washington.edu/users/krughoff/data/c4d_130830_040651_ooi_g_d1.fits.fz

        Attachments

          Issue Links

            Activity

            Hide
            swinbank John Swinbank added a comment -

            ...heads up that the version of log4cxx in the stack does not build on OSX...

            Actually, I fixed that in DM-2581, for some value of fixed – it builds, but the tests fail. That's why I say this'll be "interesting", as in "interesting times"!

            The version in the 10_1_rc3 release definitely won't build on OSX, though, and it's likely that will be the next release; any changes here should definitely not land until that's gone out.

            Show
            swinbank John Swinbank added a comment - ...heads up that the version of log4cxx in the stack does not build on OSX... Actually, I fixed that in DM-2581 , for some value of fixed – it builds, but the tests fail. That's why I say this'll be "interesting", as in "interesting times"! The version in the 10_1_rc3 release definitely won't build on OSX, though, and it's likely that will be the next release; any changes here should definitely not land until that's gone out.
            Hide
            swinbank John Swinbank added a comment -

            This is now ready for review on afw's tickets/DM-2599. Simon, would you mind taking a look?

            I found there were quite a few corner cases in the simple scheme you outlined above. I wrote a moderately extensive test to cover them, but do keep an eye out for anything I missed.

            Per K-T's advice, I didn't use lsst::log, but rather pex::logging (although I did get as far as getting the former running on OS X without any obvious errors). I did create a separate logger so that the warnings can be filtered if they become irritating (I much prefer this to adding more ctor arguments).

            Show
            swinbank John Swinbank added a comment - This is now ready for review on afw 's tickets/DM-2599 . Simon, would you mind taking a look? I found there were quite a few corner cases in the simple scheme you outlined above. I wrote a moderately extensive test to cover them, but do keep an eye out for anything I missed. Per K-T's advice, I didn't use lsst::log, but rather pex::logging (although I did get as far as getting the former running on OS X without any obvious errors). I did create a separate logger so that the warnings can be filtered if they become irritating (I much prefer this to adding more ctor arguments).
            Hide
            krughoff Simon Krughoff added a comment -

            The only thing I have to say is that there should be a test to make sure the MaskedImage constructor and Exposure constructor behave identically. Other than that, good to merge.

            I couldn't run the tests since it took me so long to get to it. Specifically, the change to remove mention of eups in the tests has broken tests. As of w.2025.22 this has been fixed.

            Show
            krughoff Simon Krughoff added a comment - The only thing I have to say is that there should be a test to make sure the MaskedImage constructor and Exposure constructor behave identically. Other than that, good to merge. I couldn't run the tests since it took me so long to get to it. Specifically, the change to remove mention of eups in the tests has broken tests. As of w.2025.22 this has been fixed.
            Hide
            swinbank John Swinbank added a comment -

            Thanks Simon. In fact, MaskedImage and Exposure aren't quite identical, as the latter always sets needAllHdus to False. I rearranged the tests slightly to account for that & will merge once I have a clean buildbot run.

            Show
            swinbank John Swinbank added a comment - Thanks Simon. In fact, MaskedImage and Exposure aren't quite identical, as the latter always sets needAllHdus to False . I rearranged the tests slightly to account for that & will merge once I have a clean buildbot run.
            Hide
            krughoff Simon Krughoff added a comment -

            Got it. Sounds good to me.

            Show
            krughoff Simon Krughoff added a comment - Got it. Sounds good to me.

              People

              • Assignee:
                swinbank John Swinbank
                Reporter:
                rbiswas Rahul Biswas
                Reviewers:
                Simon Krughoff
                Watchers:
                Jim Bosch, John Swinbank, Rahul Biswas, Simon Krughoff, Tim Jenness
              • Votes:
                0 Vote for this issue
                Watchers:
                5 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:

                  Summary Panel