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

Bugs in obs_subaru found by PyFlakes

    XMLWordPrintable

    Details

    • Story Points:
      1
    • Sprint:
      DRP X16-2, DRP X16-3
    • Team:
      Data Release Production

      Description

      I ran pyflakes on the code in obs_subaru and found a few bugs (beyond a few trivial ones that I am fixing as part of DM-5462)

      ingest.py has undefined name day0

      ccdTesting.py has at least three undefined variables: x, y and vig in the following:

          ngood += pupilImage[y[good], x[good]].sum()
       
      vig[i] = float(ngood)
      

      crosstalkYagi.py has many undefined names, starting with makeList, estimateCoeffs

        Attachments

          Issue Links

            Activity

            Hide
            nlust Nate Lust added a comment -

            Thanks for taking a look at this

            Show
            nlust Nate Lust added a comment - Thanks for taking a look at this
            Hide
            rearmstr Bob Armstrong added a comment -

            Changes look fine to me.

            Show
            rearmstr Bob Armstrong added a comment - Changes look fine to me.
            Hide
            swinbank John Swinbank added a comment -

            Doesn't seem like the changes here address everything in the ticket, though. For example, just taking:

            crosstalkYagi.py has many undefined names, starting with makeList, estimateCoeffs

            I still see:

            [jds@magpie obs_subaru (tickets/DM-5474)]$ pyflakes-2.7 ./python/lsst/obs/subaru/crosstalkYagi.py
            ./python/lsst/obs/subaru/crosstalkYagi.py:35: 'sys' imported but unused
            ./python/lsst/obs/subaru/crosstalkYagi.py:36: 'math' imported but unused
            ./python/lsst/obs/subaru/crosstalkYagi.py:38: 'time' imported but unused
            ./python/lsst/obs/subaru/crosstalkYagi.py:277: undefined name 'printCoeffs'
            ./python/lsst/obs/subaru/crosstalkYagi.py:279: undefined name 'readImage'
            ./python/lsst/obs/subaru/crosstalkYagi.py:281: undefined name 'subtractXTalk'
            

            Show
            swinbank John Swinbank added a comment - Doesn't seem like the changes here address everything in the ticket, though. For example, just taking: crosstalkYagi.py has many undefined names, starting with makeList , estimateCoeffs I still see: [jds@magpie obs_subaru (tickets/DM-5474)]$ pyflakes-2.7 ./python/lsst/obs/subaru/crosstalkYagi.py ./python/lsst/obs/subaru/crosstalkYagi.py:35: 'sys' imported but unused ./python/lsst/obs/subaru/crosstalkYagi.py:36: 'math' imported but unused ./python/lsst/obs/subaru/crosstalkYagi.py:38: 'time' imported but unused ./python/lsst/obs/subaru/crosstalkYagi.py:277: undefined name 'printCoeffs' ./python/lsst/obs/subaru/crosstalkYagi.py:279: undefined name 'readImage' ./python/lsst/obs/subaru/crosstalkYagi.py:281: undefined name 'subtractXTalk'
            Hide
            rowen Russell Owen added a comment -

            I'm also finding pyflakes warnings in most of the .py files in bin.src, though I did not bother to try to estimate the severity. I hope you will fix all pyflakes warnings and errors in all python files in obs_subaru on this ticket.

            If you don't already have a python linter extension/plugin installed in your text editor then please consider installing one. I find them very useful for catching issues early and if nothing else it would be a big help for this ticket.

            Show
            rowen Russell Owen added a comment - I'm also finding pyflakes warnings in most of the .py files in bin.src, though I did not bother to try to estimate the severity. I hope you will fix all pyflakes warnings and errors in all python files in obs_subaru on this ticket. If you don't already have a python linter extension/plugin installed in your text editor then please consider installing one. I find them very useful for catching issues early and if nothing else it would be a big help for this ticket.
            Hide
            swinbank John Swinbank added a comment -

            Nate Lust & I discussed the scope of this ticket earlier.

            We're agreed that there are a lot of linter errors in obs_subaru, and we should eliminate them. However, this effort we've allocated to this ticket was on the basis of (fully, i.e. more so than now!) cleaning up the errors listed in the ticket description, rather than finding and fixing all possible errors throughout the package. Furthermore, since we expect to be able to eliminate some of the package's code (DM-4740), it wouldn't make sense to spend time on fixes before that ticket is implemented.

            Therefore, Nate will fix the three errors listed in this ticket description and file a new ticket, blocked by DM-4740, to eliminate further errors.

            Show
            swinbank John Swinbank added a comment - Nate Lust & I discussed the scope of this ticket earlier. We're agreed that there are a lot of linter errors in obs_subaru, and we should eliminate them. However, this effort we've allocated to this ticket was on the basis of (fully, i.e. more so than now!) cleaning up the errors listed in the ticket description, rather than finding and fixing all possible errors throughout the package. Furthermore, since we expect to be able to eliminate some of the package's code ( DM-4740 ), it wouldn't make sense to spend time on fixes before that ticket is implemented. Therefore, Nate will fix the three errors listed in this ticket description and file a new ticket, blocked by DM-4740 , to eliminate further errors.
            Hide
            rearmstr Bob Armstrong added a comment -

            The additional changes look fine to me.

            Show
            rearmstr Bob Armstrong added a comment - The additional changes look fine to me.

              People

              Assignee:
              nlust Nate Lust
              Reporter:
              rowen Russell Owen
              Reviewers:
              Bob Armstrong
              Watchers:
              Bob Armstrong, John Swinbank, Lauren MacArthur, Nate Lust, Russell Owen
              Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved:

                  CI Builds

                  No builds found.