More renaming of instance #925
Conversation
| ## `kaggle models variations init` | ||
|
|
||
| Initializes a metadata file (`model-instance-metadata.json`) for creating a new model variation. | ||
| Note that the name of the file reflects the old name for a variation, which was "instance". |
There was a problem hiding this comment.
Should we add support for either model-instance-metadata.json or model-variation-metadata.json. The latter always having priority over the former. And documentation should mention using model-variation-metadata.json with maybe a small note about the former name.
Can be addressed in a separate PR.
There was a problem hiding this comment.
Not a bad idea. I'm planning to not change any more code for this release.
There was a problem hiding this comment.
On the flip-side, having a new metadata filename (even though would not be a breaking if implemented as described above) might be better to do at a major version bump.
Agree that we want to limit scope creep but can be likely addressed in a separate PR of <10 lines of code.
| # sed -i 's/INSERT_OWNER_SLUG_HERE/your-username/' tmp/model-instance-metadata.json | ||
| # sed -i 's/INSERT_EXISTING_MODEL_SLUG_HERE/parent-model-slug/' tmp/model-instance-metadata.json | ||
| # sed -i 's/INSERT_INSTANCE_SLUG_HERE/my-instance-slug/' tmp/model-instance-metadata.json | ||
| # sed -i 's/INSERT_INSTANCE_SLUG_HERE/my-variation-slug/' tmp/model-instance-metadata.json |
There was a problem hiding this comment.
Should it be INSERT_VARIATION_SLUG? (may need to update the init command to set the proper value).
There was a problem hiding this comment.
That would require a change in the code.
README.md
Outdated
|
|
||
| To run integration tests on your local machine, you need to set up your Kaggle API credentials. You can do this in one of these two ways described [this doc](docs/README.md). Refer to the sections: | ||
| To run integration tests on your local machine, you need to set up your Kaggle credentials. You can do this in one of these two ways described [this doc](docs/README.md). Refer to the sections: | ||
| - Using environment variables |
There was a problem hiding this comment.
The sections have been renamed. We now have 3 ways. I wouldn't list then here given more may be added (like OAuth).
I would simply change the link & phrasing:
You can do this by following the [authentication instructions](docs#authentication).
docs/models_metadata.md
Outdated
| * `overview`: a short overview of the instance | ||
| * `usage`: the instance's usage in markdown syntax (see the template below) | ||
| * `fineTunable`: whether the instance is fine tunable | ||
| * `variationSlug`: the slug of the variation |
There was a problem hiding this comment.
Did you also change the metadata field name? Changing this would break everyone with existing model-instance-metadata.json file. Unless we support either.
There was a problem hiding this comment.
Missed that one when I was undoing changes that affect code.
Was missing the anchor as discussed here: #925 (comment)
2 participants
No description provided.