Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Clarification] Precedence of helptext generation. #242

Closed
emcd opened this issue Jan 26, 2025 · 2 comments · Fixed by #246
Closed

[Clarification] Precedence of helptext generation. #242

emcd opened this issue Jan 26, 2025 · 2 comments · Fixed by #246

Comments

@emcd
Copy link
Contributor

emcd commented Jan 26, 2025

Comments on lines above attributes take precedence over the docstrings on the classes for those attributes. Example:

@dataclass
class Cli:
    application: ApplicationInformation
    configfile: Optional[ str ] = None
    # display: ConsoleDisplay
    inscription: InscriptionControl = (
        InscriptionControl( mode = InscriptionModes.Rich ) )
    command: Union[
        Annotated[
            _create.Command,
            tyro.conf.subcommand( # pyright: ignore
                'create', prefix_name = False ),
        ],
        Annotated[
            _apply.Command,
            tyro.conf.subcommand( # pyright: ignore
                'apply', prefix_name = False ),
        ],
    ]

results in:

╭─ options ────────────────────────────────────────────────────────────────────╮
│ -h, --help              show this help message and exit                      │
│ --configfile {None}|STR                                                      │
│                         (default: None)                                      │
╰──────────────────────────────────────────────────────────────────────────────╯
╭─ application options ────────────────────────────────────────────────────────╮
│ Information about an application.                                            │
│ ──────────────────────────────────────────────────────────────────────────── │
│ --application.name STR  (default: foo)                                       │
╰──────────────────────────────────────────────────────────────────────────────╯
╭─ inscription options ────────────────────────────────────────────────────────╮
│ display: ConsoleDisplay                                                      │
│ ──────────────────────────────────────────────────────────────────────────── │
│ --inscription.mode {null,pass,rich}                                          │
│                         (default: rich)                                      │
│ --inscription.level {None,debug,info,warn,error,critical}                    │
│                         (default: None)                                      │
╰──────────────────────────────────────────────────────────────────────────────╯
╭─ subcommands ────────────────────────────────────────────────────────────────╮
│ display: ConsoleDisplay                                                      │
│ ──────────────────────────────────────────────────────────────────────────── │
│ {create,apply} 

However, similar to ApplicationInformation, InscriptionControl has a docstring on the class:

@dataclass
class ApplicationInformation:
    ''' Information about an application. '''
@dataclass
class InscriptionControl:
    ''' Logging and debug printing behavior. '''

But, only the docstring for ApplicationInformation shows correctly. The docstring for InscriptionControl is occluded by the commented field.

Is this desired behavior? If so, should it be clarified in https://brentyi.github.io/tyro/helptext_generation/?

Also, would you consider it useful to have another tyro.conf annotation to suppress this behavior? As someone who frequently scaffolds code and leaves in TODO comments, I've been annoyed by scaffolding comments showing up as helptext on more than one occasion.

@brentyi
Copy link
Owner

brentyi commented Jan 28, 2025

Hi @emcd! This is as-designed, but I see why it's annoying. The reason is that the comment is treated as a "docstring" for the field, and we treat field docstrings as "more specific" than class docstrings.

For moving forward: adding a flag to turn off comment scraping is non-invasive and I'm happy to do it. We could also revisit the specific default behaviors, although my guess is that all possible choices here just leads to different tradeoffs and there's no objective right thing to do.

@emcd
Copy link
Contributor Author

emcd commented Jan 28, 2025

@brentyi : You have an amazing turnaround time. Thanks for the PR. I was going to offer to make it if you were amenable to the new flag, but am not going to complain if the project maintainer spontaneously produces patches instead. :)

To be clear, I am not interested in changing the defaults. I agree that there is no objective right thing to do here. Was mainly seeking clarification on the order of precedence... attribute-specific over class docstring makes sense. I don't recall that being explicitly mentioned in the documentation on helptext generation though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants