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

Add "disallow if False:" note to developer docs

    XMLWordPrintable

    Details

    • Story Points:
      0.5
    • Epic Link:
    • Sprint:
      Alert Production S17 - 1
    • Team:
      Alert Production

      Description

      This is the implementation ticket for RFC-205.

      The suggested additional text is given below, which probably best fits as a subsection of section 10 of the python style guide:

      Code must not be placed inside `if False:` or `if True:` blocks, nor left commented out. If one has debugging code or if one is undecided about which particular implementation to use, such code must be placed inside a "named" `if` statement. Such blocks may have a comment describing the conditions under which said code can be removed (e.g. completion of a ticket, a particular date). For example, for code that will likely be removed in the future, once testing is completed:

      # Delete old_thing() and this "if" once we have all the unittests in place (DM-123456).
      use_old_method = False
      if use_old_method:
          old_thing()
      else:
          new_thing()
      

      Such debugging flags should usually be lifted up into the method's keyword arguments to allow users to decide which branch to run. For example:

      def foo(x, debug_plots=False):
          do_thing()
          if debug_plots:
              plot_thing()
      

      or, using lsstDebug, which can be controlled as part of a commandline task:

      import lsstDebug
      def foo(x):
          do_thing()
          if lsstDebug.Info(__name__).debug_plots:
              plot_thing()
      

        Attachments

          Issue Links

            Activity

            Hide
            Parejkoj John Parejko added a comment -

            Doing this while awaiting reviews.

            Show
            Parejkoj John Parejko added a comment - Doing this while awaiting reviews.
            Hide
            Parejkoj John Parejko added a comment -

            Colin: are you happy with the wording of this, as implemented in the developer guide branch?

            Jonathan: does this look like an appropriate section to put this in, and do you have any ReST formatting suggestions?

            Thanks to both of you.

            Show
            Parejkoj John Parejko added a comment - Colin: are you happy with the wording of this, as implemented in the developer guide branch ? Jonathan: does this look like an appropriate section to put this in, and do you have any ReST formatting suggestions? Thanks to both of you.
            Hide
            ctslater Colin Slater added a comment -

            Looks good to me.

            Show
            ctslater Colin Slater added a comment - Looks good to me.
            Hide
            jsick Jonathan Sick added a comment -

            Looks great. I provided some feedback in the PR that you can consider before merging.

            Thanks for getting this into the Python style guide!

            Show
            jsick Jonathan Sick added a comment - Looks great. I provided some feedback in the PR that you can consider before merging. Thanks for getting this into the Python style guide!
            Hide
            Parejkoj John Parejko added a comment -

            Thank you for the comments: I cleaned things up and merged.

            Show
            Parejkoj John Parejko added a comment - Thank you for the comments: I cleaned things up and merged.

              People

              Assignee:
              Parejkoj John Parejko
              Reporter:
              Parejkoj John Parejko
              Reviewers:
              Jonathan Sick
              Watchers:
              Colin Slater, John Parejko, Jonathan Sick, Kian-Tat Lim, Simon Krughoff
              Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved:

                  CI Builds

                  No builds found.