Skip to content

ExperimentalAPI CLI#428

Open
timmarkhuff wants to merge 30 commits intomainfrom
tim/experimental-cli
Open

ExperimentalAPI CLI#428
timmarkhuff wants to merge 30 commits intomainfrom
tim/experimental-cli

Conversation

@timmarkhuff
Copy link
Copy Markdown
Contributor

@timmarkhuff timmarkhuff commented Apr 27, 2026

  1. adds support for ExperimentalApi methods in CLI
  2. converts return values into human-readable strings
  3. introduces a mechanism to hide certain functions from the CLI (either because they cannot return a value that is human readable, or because they just don't belong in the CLI)
  4. entirely removes certain functions from the ExperimentalApi that needed to be deprecated.
  5. organizes CLI functions into groupings and alphabetizes them

@timmarkhuff timmarkhuff marked this pull request as draft April 28, 2026 16:36
Comment thread src/groundlight/cli.py
bound_method = getattr(gl, fake_method.__name__)
result = bound_method(*args, **kwargs)
if result is not None:
print(_format_result(result))
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Heavily biased as the one who wrote the original, but there should be more comments around this one line. This is the magic line that makes the whole thing happen

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. I added a comment.

Comment thread src/groundlight/cli.py

# Maps method names to their rich_help_panel group label for the CLI help output.
# Applies to both stable and experimental commands.
_COMMAND_GROUPS: dict[str, str] = {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this adds maintenance overhead, new methods need to belong to a group. The original design tried to avoid that additional overhead, but in the age of AI maybe we don't care

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a test to ensure that each method has a CLI group. It will be an extra step, but a worthwhile extra step that cannot be forgotten.

@timmarkhuff timmarkhuff marked this pull request as ready for review May 5, 2026 23:59
@timmarkhuff timmarkhuff requested a review from brandon-wada May 5, 2026 23:59
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 this pull request may close these issues.

2 participants