Skip to content
This repository has been archived by the owner on Oct 16, 2024. It is now read-only.

Semi-autogenerated cli reference #172

Merged
merged 17 commits into from
Oct 5, 2022
Merged

Semi-autogenerated cli reference #172

merged 17 commits into from
Oct 5, 2022

Conversation

mike0sv
Copy link
Contributor

@mike0sv mike0sv commented Sep 3, 2022

First step for #171

This PR contains auto-generation machinery for CLI reference.

_generate_cli_spec.py generates spec.json file from mlem.cli.app object
_generate_options.py reads spec.json and for each command generates sections and inserts them into respective .md files

Auto-generated sections:

  • first paragraph (between cmd name and synopsys): mlem cli help message
  • usage
    • usage text
    • arguments with help
    • subcommands (for groups)
  • options: list of cli options with help

There are also 2 not generated sections: Description and Examples.

Example: suppose we have a key in spec.json

"apply": {
    "args": {
      "args": [
        {
          "decl": "model",
          "help": "Path to model object  [required]"
        }
      ],
      "impls": null,
      "impl_metavar": null,
      "subcommands": null
    },
    "options": [
      {
        "decl": "-p, --project TEXT",
        "help": "Path to MLEM project  [default: (none)]"
      }
    ],
    "usage": "Usage: mlem apply [options] model data",
    "doc": "Apply a model to data. The result will be saved as a MLEM object to `output` if\nprovided. Otherwise, it will be printed to `stdout`."
  }

And apply.md

# apply

AAAA

## Synopsys

`` `usage
BBB
`` `

## Description

CCC

## Options

DDD

## Examples

EEE

It will be transformed in place into:

# apply

Apply a model to data. The result will be saved as a MLEM object to `output` if
provided. Otherwise, it will be printed to `stdout`.

## Synopsis

`` `usage
Usage: mlem apply [options] model data

Arguments:
- `model`: Path to model object  [required]
`` `

## Description

CCC

## Options

- `-p, --project TEXT`: Path to MLEM project  [default: (none)]

## Examples

EEE

Feel free to suggest changes to generated content formatting and to spec schema

@shcheklein shcheklein temporarily deployed to mlem-ai-new-cli-qsq7njeufcmrzh September 3, 2022 08:58 Inactive
@github-actions
Copy link

github-actions bot commented Sep 3, 2022

095acea

Link Check Report

All 3 links passed!

CML watermark

@shcheklein shcheklein temporarily deployed to mlem-ai-new-cli-qsq7njeufcmrzh September 5, 2022 06:03 Inactive
@shcheklein shcheklein temporarily deployed to mlem-ai-new-cli-qsq7njeufcmrzh September 5, 2022 06:07 Inactive
@shcheklein shcheklein temporarily deployed to mlem-ai-new-cli-qsq7njeufcmrzh September 5, 2022 06:20 Inactive
@casperdcl
Copy link

arguments:
MODEL Path to model [required]
[SUBTYPE] Type of build. Choices: ['whl', 'pip', 'docker_dir', 'docker']
Builtin builders:
Copy link
Member

Choose a reason for hiding this comment

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

MODEL -> model

required / optional check how it's done in other docs

list with options - it was better the way it was done (compact, easier to read, etc)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

required and optional are different things: required means you always should provide it, optional means it can have None value. Required optional means you will get error if you don't provide it, but you can set it to None. Not required not optional means there is a default value which is not None and you can't set it to None

also cli changed a bit, now mlem build * are subcommands (instead of argument). Also mlem build is a separate command (for building from saved config using -l). I will update this soon when I decide how to better reflect this (same with serve and apply-remote)


## Options

- `-p, --project TEXT`: Path to MLEM project [default: (none)]
Copy link
Member

Choose a reason for hiding this comment

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

style: -p <path>, --project <path> - path to MLEM ...

default - check how it's done in the docs

TEXT should be renamed into path, also in the CLI itself

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done, but sometimes the duplication of metavar feels strange

Copy link
Contributor

@jorgeorpinel jorgeorpinel left a comment

Choose a reason for hiding this comment

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

A few misc. Qs below.

I like the intent but I'm a bit skeptical about actually applying this as a process. Curious to see how it goes though, happy to help if I can. Lmk!

content/docs/command-reference/spec.json Outdated Show resolved Hide resolved
content/docs/command-reference/spec.json Outdated Show resolved Hide resolved
content/docs/command-reference/apply.md Outdated Show resolved Hide resolved
content/docs/command-reference/spec.json Outdated Show resolved Hide resolved
content/docs/command-reference/checkenv.md Show resolved Hide resolved
@shcheklein shcheklein temporarily deployed to mlem-ai-new-cli-qsq7njeufcmrzh September 9, 2022 03:05 Inactive
@shcheklein shcheklein temporarily deployed to mlem-ai-new-cli-qsq7njeufcmrzh September 9, 2022 03:27 Inactive
@shcheklein shcheklein temporarily deployed to mlem-ai-new-cli-qsq7njeufcmrzh September 13, 2022 10:32 Inactive
@shcheklein shcheklein temporarily deployed to mlem-ai-new-cli-qsq7njeufcmrzh September 13, 2022 10:45 Inactive
@shcheklein shcheklein temporarily deployed to mlem-ai-new-cli-qsq7njeufcmrzh September 13, 2022 10:48 Inactive
Copy link
Contributor

@jorgeorpinel jorgeorpinel left a comment

Choose a reason for hiding this comment

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

I think that the automation effort is always great and this particular PR is def. an improvement and quite mergeable (after removing script and auxiliary files). Some minor comments below, but I'm not blocking it.

I'm not sure that we can continue to use this specific script though, maybe start only with overwriting usage blocks.

Cc @omesser FYI (rel. #171 (comment))

Comment on lines +20 to +21

## Examples
Copy link
Contributor

Choose a reason for hiding this comment

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

No need for these lines when there are no examples.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah, I added this to not forget to add them :)

content/docs/command-reference/config.md Show resolved Hide resolved
content/docs/command-reference/config.md Outdated Show resolved Hide resolved
content/docs/command-reference/init.md Show resolved Hide resolved
List [MLEM objects](/doc/user-guide/basic-concepts#mlem-objects) inside a MLEM
project (location should be [initialized](/doc/command-reference/init)).

> Aliased to `mlem ls`
List MLEM objects inside a [MLEM project](/doc/user-guide/project-structure).
Copy link
Contributor

Choose a reason for hiding this comment

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

We're losing a link and the alias note here.

Comment on lines 1 to +3
# serve

Locally deploy the model using a server implementation and expose its methods as
endpoints.
Create an API from model methods using a server implementation.
Copy link
Contributor

Choose a reason for hiding this comment

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

Previous version may be better? (would need core change)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I changed this because you commented on core repo PR :) So should I return old doc?

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, I don't remember the context. Can you link to the discussion? Up to you though, thanks.

@shcheklein shcheklein temporarily deployed to mlem-ai-new-cli-qsq7njeufcmrzh September 19, 2022 12:45 Inactive
@shcheklein shcheklein temporarily deployed to mlem-ai-new-cli-qsq7njeufcmrzh September 19, 2022 12:50 Inactive
@mike0sv mike0sv changed the base branch from main to release/0.3.0 September 19, 2022 12:51
@shcheklein shcheklein temporarily deployed to mlem-ai-new-cli-qsq7njeufcmrzh September 19, 2022 12:58 Inactive
@jorgeorpinel
Copy link
Contributor

Should mlem dev be included @mike0sv? I'm curious why that command was not generated. Rel. #180.

Also, are there clear next steps to move this forward or are we stuck somewhere? Thanks

@jorgeorpinel jorgeorpinel mentioned this pull request Sep 21, 2022
3 tasks
@jorgeorpinel jorgeorpinel added A: docs Area: user documentation (gatsby-theme-iterative) C: ref Content of /doc/{command,api,object}-reference labels Sep 21, 2022
@aguschin aguschin mentioned this pull request Sep 23, 2022
18 tasks
@mike0sv
Copy link
Contributor Author

mike0sv commented Oct 4, 2022

I'd say lets merge this! I created a separate release/0.3.0 branch just like in core repo so we can finalize bits of work on new release docs without going live with them

@aguschin aguschin merged commit 0525959 into release/0.3.0 Oct 5, 2022
@aguschin aguschin deleted the new-cli branch October 5, 2022 11:10
aguschin added a commit to iterative/mlem that referenced this pull request Oct 27, 2022
This is a list of activities that should be done before release announce

## First, things before release

### Breaking changes - merge in this PR
- [x] New deployment state management
- [x] New CLI interface: breaking changes in `serve`, `apply-remote`,
`build` and `declare`

### Other changes - merge in this PR
- [x] SageMaker deployments
- [x] K8s deployments
- [x] `--quiet` #448 @mike0sv 

### Docs
- [x] Finish semi-generated API reference PR
iterative/mlem.ai#172
- [x] Update Quick Start in this repo
- [x] iterative/mlem.ai#197
- [ ] iterative/mlem.ai#188

### Blog post
- [ ] blog post re release @aguschin 

### Example-repo - PR in `example-repos-dev`
- [x] update `example-mlem-get-started`

### Update the website code animations
- [x] https://mlem.ai

## Second, the release itself

### MLEM Release
- [ ] merge this branch
- [ ] release it on PyPi
- [ ] merge docs PR
- [ ] merge PR in `example-repos-dev` and update
`iterative/example-mlem-get-started`

## Third, things after release

- [ ] remove `xfail` mark from
`tests/cli/test_mail.py::test_commands_docs_links`
@jorgeorpinel jorgeorpinel added the dependencies Pull requests that update a dependency file label Oct 30, 2022
@jorgeorpinel jorgeorpinel added the ✨ epic Placeholder ticket for multi-sprint direction, use story, improvement label Dec 2, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A: docs Area: user documentation (gatsby-theme-iterative) C: ref Content of /doc/{command,api,object}-reference dependencies Pull requests that update a dependency file ✨ epic Placeholder ticket for multi-sprint direction, use story, improvement
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

5 participants