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

HSC backport: Include documentation strings for config parameters when they are dumped

    Details

    • Type: Story
    • Status: Done
    • Resolution: Done
    • Fix Version/s: None
    • Component/s: pex_config, pipe_base
    • Labels:
      None

      Description

      This is a port of the following HSC tickets:
      HSC-1072
      and
      HSC-1175

        Attachments

          Issue Links

            Activity

            Hide
            ktl Kian-Tat Lim added a comment - - edited

            (This discussion should really have gone somewhere else, I think, but it's probably too late now.) Robert Lupton: It is approaching two years since we adopted from __future__ import print_function (see official policy and original adoption). Is there some technical or human-factors reason to make an exception here beyond "I don't think that's acceptable", "is a bad idea", and "I'd rather not force them"? Do we have a large external population of print-in-config-override writers, for example? (There are only 10 occurrences in our code base, many copies of the same original.)

            Show
            ktl Kian-Tat Lim added a comment - - edited (This discussion should really have gone somewhere else, I think, but it's probably too late now.) Robert Lupton : It is approaching two years since we adopted from __future__ import print_function (see official policy and original adoption ). Is there some technical or human-factors reason to make an exception here beyond "I don't think that's acceptable", "is a bad idea", and "I'd rather not force them"? Do we have a large external population of print-in-config-override writers, for example? (There are only 10 occurrences in our code base, many copies of the same original.)
            Hide
            tjenness Tim Jenness added a comment -

            I think "all users" and "configs supported by LSST" are two different things. Since the user is importing configs as standard python code using import then they just need to write a config that works on their python. We have to ship configs that will work on all the python versions we support and using our internal coding standards (the price we pay for configs being python code rather than YAML).

            Show
            tjenness Tim Jenness added a comment - I think "all users" and "configs supported by LSST" are two different things. Since the user is importing configs as standard python code using import then they just need to write a config that works on their python. We have to ship configs that will work on all the python versions we support and using our internal coding standards (the price we pay for configs being python code rather than YAML).
            Hide
            ktl Kian-Tat Lim added a comment - - edited

            The config override files are ultimately evaluated by pex_config code, even if they are imported by other config files. Config override files should never be directly imported by standard Python code. So I believe the config override files need to match the language environment in effect at this line, possibly as affected by flags or dont_inherit arguments. The question is whether this file (or this line) should be special-cased.

            Show
            ktl Kian-Tat Lim added a comment - - edited The config override files are ultimately evaluated by pex_config code, even if they are imported by other config files. Config override files should never be directly imported by standard Python code. So I believe the config override files need to match the language environment in effect at this line , possibly as affected by flags or dont_inherit arguments. The question is whether this file (or this line) should be special-cased.
            Hide
            rhl Robert Lupton added a comment -

            The reason for my concern is that end-users will get syntax errors from valid python because we have decided to make python2 look like python3.

            In this case I am willing to deal with the consequences of this change upon e.g. HSC open use observers once we switch to running LSST code in production on HSC. There are probably not many of them; let us hope this is true.

            Show
            rhl Robert Lupton added a comment - The reason for my concern is that end-users will get syntax errors from valid python because we have decided to make python2 look like python3. In this case I am willing to deal with the consequences of this change upon e.g. HSC open use observers once we switch to running LSST code in production on HSC. There are probably not many of them; let us hope this is true.
            Hide
            ktl Kian-Tat Lim added a comment -

            We have decided to use a dialect of Python 2 that looks and works a little like Python 3. The only alternative I can think of is to catch SyntaxError exceptions from compile and retry with only flags=__future__.CO_FUTURE_ABSOLUTE_IMPORT|__future__.CO_FUTURE_DIVISION and dont_inherit=1, which would allow configs to use either style.

            (Note: in my python 2.7.10, the documentation's claim that "The bitfield required to specify a given feature can be found as the compiler_flag attribute on the _Feature instance in the __future__ module" appears to be false.)

            Show
            ktl Kian-Tat Lim added a comment - We have decided to use a dialect of Python 2 that looks and works a little like Python 3. The only alternative I can think of is to catch SyntaxError exceptions from compile and retry with only flags=__future__.CO_FUTURE_ABSOLUTE_IMPORT|__future__.CO_FUTURE_DIVISION and dont_inherit=1 , which would allow configs to use either style. (Note: in my python 2.7.10, the documentation's claim that "The bitfield required to specify a given feature can be found as the compiler_flag attribute on the _Feature instance in the __future__ module" appears to be false.)

              People

              • Assignee:
                lauren Lauren MacArthur
                Reporter:
                lauren Lauren MacArthur
                Reviewers:
                John Swinbank
                Watchers:
                David Nidever [X] (Inactive), Frossie Economou, John Swinbank, Jonathan Sick, Kian-Tat Lim, Lauren MacArthur, Paul Price, Robert Lupton, Russell Owen, Tim Jenness
              • Votes:
                0 Vote for this issue
                Watchers:
                10 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:

                  Summary Panel