sacred icon indicating copy to clipboard operation
sacred copied to clipboard

Multi-line config comments

Open bzamecnik opened this issue 6 years ago • 3 comments

Sacred can pick comments for config arguments, but only single lines. The reason is that it shows the comment after the argument on the same line.

It would be nice if it can pick even multi-line comments, so that the comment doesn't have to be a single long line. In such case it print the comment before the argument, the same way it's in the code.

from sacred import Experiment

ex = Experiment('multiline')

@ex.config
def my_config():
    # foo comment
    foo = 1
    # multiline
    # bar comment
    bar = 2

@ex.automain
def main():
    pass

Actual output:

$ python multiline.py print_config
INFO - multiline - Running command 'print_config'
INFO - multiline - Started
Configuration (modified, added, typechanged, doc):
  bar = 2                            # bar comment
  foo = 1                            # foo comment
  seed = 718098747                   # the random seed for this experiment
INFO - multiline - Completed after 0:00:00

Expected output:

$ python multiline.py print_config
INFO - multiline - Running command 'print_config'
INFO - multiline - Started
Configuration (modified, added, typechanged, doc):
  # multiline
  # bar comment
  bar = 2
  foo = 1                            # foo comment
  seed = 718098747                   # the random seed for this experiment
INFO - multiline - Completed after 0:00:00

bzamecnik avatar Feb 08 '19 12:02 bzamecnik

I agree that multiline comments might be nice. Though I think I'd prefer the format:

$ python multiline.py print_config
INFO - multiline - Running command 'print_config'
INFO - multiline - Started
Configuration (modified, added, typechanged, doc):
  bar = 2                            # multiline
                                       bar comment
  foo = 1                            # foo comment
  seed = 718098747                   # the random seed for this experiment
INFO - multiline - Completed after 0:00:00

Also there should then probably be a way to choose between a short and a verbose version of the config. Maybe the default could be this:

$ python multiline.py print_config
INFO - multiline - Running command 'print_config'
INFO - multiline - Started
Configuration (modified, added, typechanged, doc):
  bar = 2                            # multiline [...]
  foo = 1                            # foo comment
  seed = 718098747                   # the random seed for this experiment
INFO - multiline - Completed after 0:00:00

All of this should not be too hard to implement. It should be enough to modify this function to gather multiple comment lines.

Qwlouse avatar Feb 19 '19 11:02 Qwlouse

We might want to think about the exact syntax for this. Some cases are obvious, but some other might confuse the users, like discussed in the rejected PEP 258 (https://www.python.org/dev/peps/pep-0258/#attribute-docstrings). If we allow all cases shown below, there are some ambiguous cases if the newlines are omitted. If we just allow the multiline comments with # before the variable, everything should be fine. But, some users might be confused if other documenting styles are not supported.

@ex.config
def config():
    # Single line docstring before the value
    a = 0

    # Long docstring
    # before
    b = 1

    c = 2 # Short docstring in line

    d = 3  # Long docstring in line
    # could be ambiguous if the next value definition follow immediately
    whats_the_docstring_of_this = '?'

    e = 4
    """One-line function-like docstring after the value like PEP 258"""

    f = 5
    """
    Multi-line function-like docstring,
    like in PEP 258
    """

    """Do we allow docstrings with triple qotation marks?"""
    g = 6

thequilo avatar Feb 27 '19 12:02 thequilo

Good point. I would be ok with both: only supporting multiline comments before a variable and a more comprehensive solution. But in the latter case I agree that we should be very clear on the supported syntax.

Qwlouse avatar Mar 05 '19 16:03 Qwlouse