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

Add meas_extensions_trailedSources to lsst_distrib

    XMLWordPrintable

Details

    • Story
    • Status: Done
    • Resolution: Done
    • None
    • None
    • None
    • 21
    • AP S21-5 (April), AP S21-6 (May)
    • Alert Production
    • No

    Description

      Implement trailed sources measurement extension from RFC-768.

      Attachments

        Issue Links

          Activity

            sullivan Ian Sullivan added a comment - Note: the code can be found in https://github.com/lsst-dm/meas_extensions_trailedSources

            cmorrison Please check the contents of doc/ and the built documentation.
            krzys Please check contents of include/, lib/, src/, and ups/ as well as the pybind11 wrappers in python/lsst/meas/extensions/trailedSources/
            gkovacs Please check the python code in python/ and the contents of tests/

            After you have done your review, remove your name from the Reviewers list.

            Thank you very much!

            sullivan Ian Sullivan added a comment - cmorrison Please check the contents of doc/ and the built documentation. krzys Please check contents of include/ , lib/ , src/ , and ups/ as well as the pybind11 wrappers in python/lsst/meas/extensions/trailedSources/ gkovacs Please check the python code in python/ and the contents of tests/ After you have done your review, remove your name from the Reviewers list. Thank you very much!
            langfzac Zach Langford [X] (Inactive) added a comment - The code has been moved to the lsst domain: https://github.com/lsst/meas_extensions_trailedSources

            Confirmed that the docs build properly and render reasonably okay. Assuming this package never has a task or pipeline task, should those subsections of the Sphinx documentation be removed? This is a question for the other reviewers as well.

            cmorrison Chris Morrison [X] (Inactive) added a comment - - edited Confirmed that the docs build properly and render reasonably okay. Assuming this package never has a task or pipeline task, should those subsections of the Sphinx documentation be removed? This is a question for the other reviewers as well.

            They must at least be commented out. I personally have no problem with removing them entirely, since they can always be added back in later.

            krzys Krzysztof Findeisen added a comment - They must at least be commented out . I personally have no problem with removing them entirely, since they can always be added back in later.

            Hey krzys when I build the code against the latest weekly I get a bunch of compiler warnings regarding deprecation of Filter amongst others. Wanted to make sure you had seen those and confirmed that they were unrelated to the code under reviewer here. I assume they are.

            cmorrison Chris Morrison [X] (Inactive) added a comment - Hey krzys when I build the code against the latest weekly I get a bunch of compiler warnings regarding deprecation of Filter amongst others. Wanted to make sure you had seen those and confirmed that they were unrelated to the code under reviewer here. I assume they are.
            krzys Krzysztof Findeisen added a comment - - edited

            I assume those are from header files included indirectly through Exposure. None of the code in this package uses Filter.

            krzys Krzysztof Findeisen added a comment - - edited I assume those are from header files included indirectly through Exposure . None of the code in this package uses Filter .

            Please find my comments on github. I did not check whether all the (python) calculation formulas are algorithmically correct.

            Most of my comments are minor style and some docstring related. I think, I have found one problem in the unit test.

            gkovacs Gabor Kovacs [X] (Inactive) added a comment - Please find my comments on github. I did not check whether all the (python) calculation formulas are algorithmically correct. Most of my comments are minor style and some docstring related. I think, I have found one problem in the unit test.

            langfzac I'm happy with the update, thank you!

            gkovacs Gabor Kovacs [X] (Inactive) added a comment - langfzac I'm happy with the update, thank you!

            Hi all, thanks for your comments and thoroughness! cmorrison can you confirm that the doc building issues have been addressed?

            langfzac Zach Langford [X] (Inactive) added a comment - Hi all, thanks for your comments and thoroughness!  cmorrison  can you confirm that the doc building issues have been addressed?

            People

              langfzac Zach Langford [X] (Inactive)
              langfzac Zach Langford [X] (Inactive)
              Chris Morrison
              Chris Morrison [X] (Inactive), Gabor Kovacs [X] (Inactive), Ian Sullivan, Krzysztof Findeisen, Mario Juric, Zach Langford [X] (Inactive)
              Votes:
              0 Vote for this issue
              Watchers:
              6 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved:

                Jenkins

                  No builds found.