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

Phase out use of FilterProperty

    XMLWordPrintable

Details

    Description

      RFC-730 proposes a filter system that does not contain any metadata other than the filter names. This particularly includes wavelength information.

      Search for code that refers to the FilterProperty class or Filter::getFilterProperty, and rewrite it to depend on external sources of the same information (e.g., transmission curves for wavelength information). I am recommending that this be done before FilterLabel is added to Exposure (and therefore before FilterProperty can be deprecated) so that any lessons learned can be incorporated into the FilterLabel design.

      Attachments

        Issue Links

          Activity

            Note that DCR use of FilterProperty is being removed separately on DM-26615.

            krzys Krzysztof Findeisen added a comment - Note that DCR use of FilterProperty is being removed separately on DM-26615 .
            tjenness Tim Jenness added a comment -

            getFilterProperty usage:

            • afw tests
            • ip_diffim in DCR (to get min, max, effective wavelength)
            • obs_base to test filters have been defined properly and for gen3 serialization
            • obs_cfht/decam/subaru to get filter names for gen2 tests
            • pipe_tasks DCR test and one place to get the name.
            tjenness Tim Jenness added a comment - getFilterProperty usage: afw tests ip_diffim in DCR (to get min, max, effective wavelength) obs_base to test filters have been defined properly and for gen3 serialization obs_cfht/decam/subaru to get filter names for gen2 tests pipe_tasks DCR test and one place to get the name.

            I'm not fully pleased with the handling of FilterProperty as part of the lsst.afw.image.utils.defineFilter function, but I'm also not sure how much of that will be retained in the end.

            czw Christopher Waters added a comment - I'm not fully pleased with the handling of FilterProperty as part of the lsst.afw.image.utils.defineFilter function, but I'm also not sure how much of that will be retained in the end.
            krzys Krzysztof Findeisen added a comment - - edited

            I've done a review pass. for removing the use of getFilterProperty as a name normalizer (a use I definitely hadn't expected), for removing its use in unit tests of Filter, FilterProperty, or supporting code.

            I don't have any objections to adding TransmissionCurve as an input to utils.defineFilter; however, since that code is essentially only for backwards compatibility at this point, I don't expect the new parameter will get used.

            krzys Krzysztof Findeisen added a comment - - edited I've done a review pass. for removing the use of getFilterProperty as a name normalizer (a use I definitely hadn't expected), for removing its use in unit tests of Filter , FilterProperty , or supporting code. I don't have any objections to adding TransmissionCurve as an input to utils.defineFilter ; however, since that code is essentially only for backwards compatibility at this point, I don't expect the new parameter will get used.

            Based on the comments from the afw PR, I think I may simply withdraw and close that PR.  I cannot now get the changes in `test_exposure.py` to complete, despite a complete Jenkins test from yesterday:

            https://ci.lsst.codes/blue/organizations/jenkins/stack-os-matrix/detail/stack-os-matrix/33072/artifacts

            After running a new Jenkins test, I will merge the remaining PRs and delete the unnecessary afw branch.

            czw Christopher Waters added a comment - Based on the comments from the afw PR, I think I may simply withdraw and close that PR.  I cannot now get the changes in `test_exposure.py` to complete, despite a complete Jenkins test from yesterday: https://ci.lsst.codes/blue/organizations/jenkins/stack-os-matrix/detail/stack-os-matrix/33072/artifacts After running a new Jenkins test, I will merge the remaining PRs and delete the unnecessary afw branch.

            Looks good, thanks for making the changes!

            krzys Krzysztof Findeisen added a comment - Looks good, thanks for making the changes!

            People

              czw Christopher Waters
              krzys Krzysztof Findeisen
              Krzysztof Findeisen
              Christopher Waters, Krzysztof Findeisen, Tim Jenness
              Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved:

                Jenkins

                  No builds found.