-
Notifications
You must be signed in to change notification settings - Fork 439
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
Rename model version to a model #2267
Rename model version to a model #2267
Conversation
Important Auto Review SkippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the To trigger a single review, invoke the WalkthroughThe core change involves renaming Changes
This is a summary of changes made across various files to reflect the renaming of Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
@coderabbitai review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 10
Configuration used: .coderabbit.yaml
Files ignored due to filter (6)
- examples/e2e/configs/deployer_config.yaml
- examples/e2e/configs/inference_config.yaml
- examples/e2e/configs/train_config.yaml
- examples/quickstart/configs/inference.yaml
- examples/quickstart/configs/training_rf.yaml
- examples/quickstart/configs/training_sgd.yaml
Files selected for processing (55)
- .github/workflows/update-templates-to-examples.yml (3 hunks)
- docs/book/user-guide/advanced-guide/data-management/model-management.md (8 hunks)
- docs/book/user-guide/advanced-guide/data-management/tagging.md (1 hunks)
- docs/book/user-guide/advanced-guide/pipelining-features/fetch-metadata-within-steps.md (1 hunks)
- docs/book/user-guide/starter-guide/structuring-a-project.md (3 hunks)
- docs/book/user-guide/starter-guide/track-ml-models.md (7 hunks)
- examples/e2e/.copier-answers.yml (1 hunks)
- examples/e2e/steps/deployment/deployment_deploy.py (1 hunks)
- examples/e2e/steps/hp_tuning/hp_tuning_select_best_model.py (1 hunks)
- examples/e2e/steps/inference/inference_predict.py (2 hunks)
- examples/e2e/steps/promotion/compute_performance_metrics_on_current_data.py (2 hunks)
- examples/e2e/steps/promotion/promote_with_metric_compare.py (3 hunks)
- examples/e2e/steps/training/model_trainer.py (1 hunks)
- examples/quickstart/.copier-answers.yml (1 hunks)
- examples/quickstart/pipelines/inference.py (1 hunks)
- examples/quickstart/quickstart.ipynb (8 hunks)
- examples/quickstart/run.py (1 hunks)
- examples/quickstart/steps/model_promoter.py (1 hunks)
- src/zenml/init.py (2 hunks)
- src/zenml/artifacts/artifact_config.py (2 hunks)
- src/zenml/artifacts/external_artifact.py (2 hunks)
- src/zenml/artifacts/external_artifact_config.py (3 hunks)
- src/zenml/cli/base.py (1 hunks)
- src/zenml/cli/model.py (1 hunks)
- src/zenml/config/compiler.py (1 hunks)
- src/zenml/config/pipeline_configurations.py (2 hunks)
- src/zenml/config/pipeline_run_configuration.py (2 hunks)
- src/zenml/config/step_configurations.py (2 hunks)
- src/zenml/model/model.py (7 hunks)
- src/zenml/model/utils.py (5 hunks)
- src/zenml/models/v2/core/model.py (3 hunks)
- src/zenml/models/v2/core/model_version.py (3 hunks)
- src/zenml/new/pipelines/model_utils.py (3 hunks)
- src/zenml/new/pipelines/pipeline.py (12 hunks)
- src/zenml/new/pipelines/pipeline_context.py (1 hunks)
- src/zenml/new/pipelines/pipeline_decorator.py (4 hunks)
- src/zenml/new/steps/step_context.py (2 hunks)
- src/zenml/new/steps/step_decorator.py (5 hunks)
- src/zenml/orchestrators/step_launcher.py (5 hunks)
- src/zenml/orchestrators/step_runner.py (3 hunks)
- src/zenml/pipelines/base_pipeline.py (1 hunks)
- src/zenml/pipelines/pipeline_decorator.py (5 hunks)
- src/zenml/steps/base_step.py (11 hunks)
- src/zenml/steps/step_decorator.py (6 hunks)
- src/zenml/zen_stores/migrations/versions/4d688d8f7aff_rename_model_version_to_model.py (1 hunks)
- tests/integration/functional/artifacts/test_artifact_config.py (11 hunks)
- tests/integration/functional/cli/conftest.py (2 hunks)
- tests/integration/functional/model/test_model_version.py (21 hunks)
- tests/integration/functional/pipelines/test_pipeline_config.py (5 hunks)
- tests/integration/functional/pipelines/test_pipeline_context.py (3 hunks)
- tests/integration/functional/steps/test_model_version.py (21 hunks)
- tests/integration/functional/test_client.py (4 hunks)
- tests/integration/functional/zen_stores/test_zen_store.py (41 hunks)
- tests/integration/functional/zen_stores/utils.py (1 hunks)
- tests/unit/model/test_model_version_init.py (2 hunks)
Files skipped from review due to trivial changes (2)
- examples/e2e/.copier-answers.yml
- src/zenml/cli/base.py
Additional comments: 218
examples/quickstart/.copier-answers.yml (1)
- 2-2: The
_commit
field in.copier-answers.yml
has been updated to2023.12.18-3-g5b0d7c9
. This change seems to be a routine update to reflect the latest commit hash.tests/unit/model/test_model_version_init.py (2)
5-5: The import statement has been updated from
zenml.model.model_version
tozenml.model.model
, reflecting the renaming ofModelVersion
toModel
.22-23: The patch path for the logger has been updated to
zenml.model.model.logger
to align with the renaming changes.src/zenml/config/pipeline_run_configuration.py (1)
- 42-42: The
model_version
attribute inPipelineRunConfiguration
has been replaced withmodel
, aligning with the renaming operation across the codebase.examples/e2e/steps/hp_tuning/hp_tuning_select_best_model.py (3)
44-44: The variable
model_version
has been renamed tomodel
to reflect the new terminology.51-51: The variable
model
has been renamed tomodel_
to avoid shadowing themodel
from the step context.55-55: The variable
best_model
is now assigned the value ofmodel_
instead ofmodel
to reflect the updated variable naming.examples/quickstart/pipelines/inference.py (2)
44-44: The
model
attribute is now used to retrieve the production model artifact, replacing the previousmodel_version
usage.47-47: Similarly, the
model
attribute is used to retrieve the preprocess pipeline artifact, ensuring consistency with the renaming operation.src/zenml/__init__.py (1)
- 48-48: The import statement for
Model
has been added, replacing the previousModelVersion
import.src/zenml/new/pipelines/model_utils.py (4)
23-23: The class
NewModelVersionRequest
has been renamed toNewModelRequest
, aligning with the new naming convention.41-41: The private attribute
_model_version
has been renamed to_model
.38-47: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [44-53]
The method
model_version
has been renamed tomodel
, and the logic within the method has been updated accordingly.
- 59-72: The
update_request
method now takes aModel
object instead of aModelVersion
object, and the internal logic has been updated to reflect this change.examples/e2e/steps/inference/inference_predict.py (3)
56-56: The
model
object is now accessed directly from the step context, replacing the previousmodel_version
usage.59-61: The logic for obtaining the predictor service has been updated to use the
model
object.71-71: The predictor is now loaded from the
model
object, ensuring consistency with the renaming operation.examples/e2e/steps/deployment/deployment_deploy.py (2)
63-63: The
model
object is now used within thedeployment_deploy
function, replacing themodel_version
usage.67-68: The parameters passed to
mlflow_model_registry_deployer_step.entrypoint
have been updated to use themodel
object.examples/quickstart/steps/model_promoter.py (3)
54-54: The
model_promoter
function now usesget_step_context().model
to access the current model context.59-62: The logic for comparing model accuracy and promoting models has been updated to use the
model
object.69-73: The
set_stage
method is now called oncurrent_model
instead ofcurrent_model_version
, reflecting the updated terminology.tests/integration/functional/pipelines/test_pipeline_context.py (4)
5-5: The import statement has been updated to use
Model
instead ofModelVersion
.67-67: The
promoter_step
function now usesget_step_context().model.set_stage
to promote the model.76-83: The
producer_pipe
andconsumer_pipe
pipelines have been updated to use themodel
parameter instead ofmodel_version
.87-94: The test functions have been updated to reflect the changes made to the entities and method calls.
src/zenml/zen_stores/migrations/versions/4d688d8f7aff_rename_model_version_to_model.py (3)
19-53: The migration script contains SQL update statements to replace occurrences of
model_version
withmodel
in the database.62-73: The
upgrade
function executes the update statements to perform the renaming operation in the database.82-93: The
downgrade
function provides the reverse operation, changingmodel
back tomodel_version
in case of a rollback.src/zenml/config/pipeline_configurations.py (1)
- 43-43: The
model_version
attribute inPipelineConfigurationUpdate
has been replaced withmodel
.examples/e2e/steps/promotion/compute_performance_metrics_on_current_data.py (2)
24-24: The import statement for
Model
has been added to the file.63-64: The variables
latest_version
andcurrent_version
are now assigned using theModel
class instead ofModelVersion
.tests/integration/functional/cli/conftest.py (2)
28-28: The import statement for
Model
has been updated, replacing the previousModelVersion
import.94-94: The
pipeline
decorator now uses themodel
parameter instead ofmodel_version
.docs/book/user-guide/advanced-guide/data-management/tagging.md (3)
51-60: The documentation now correctly refers to the
Model
object instead ofModelVersion
when creating a model with tags.67-67: The
@pipeline
decorator in the example code uses themodel
parameter correctly.48-78: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [75-83]
The Python SDK examples for creating or updating models and model versions have been updated to use the
Model
class.src/zenml/new/pipelines/pipeline_context.py (1)
- 110-110: The
model_version
attribute inPipelineContext
has been replaced withmodel
.docs/book/user-guide/advanced-guide/pipelining-features/fetch-metadata-within-steps.md (1)
- 69-69: The
my_step
function now correctly usesget_step_context().model
to access model metadata.src/zenml/new/pipelines/pipeline_decorator.py (1)
- 65-65: The
pipeline
decorator now accepts amodel
parameter of typeModel
, aligning with the renaming operation.src/zenml/artifacts/external_artifact_config.py (3)
36-36: The
model_version
attribute inExternalArtifactConfiguration
has been replaced withmodel
.40-42: A root validator has been added to ensure that either
version
ormodel
is provided, but not both, which is a good practice to prevent configuration errors.70-76: The logic for fetching an artifact from the model has been updated to use the
model
object.examples/e2e/steps/training/model_trainer.py (1)
- 106-107: The variable
model_version
has been renamed tomodel_
, and the methodlog_metadata
is now called onmodel_
instead ofmodel_version
. This change aligns with the PR's objective of renamingModelVersion
toModel
across the codebase.src/zenml/artifacts/artifact_config.py (2)
25-25: The import statement for the
Model
class has been updated to reflect the new naming convention.91-108: The type hint in the
_model_version
method has been changed fromModelVersion
toModel
, and the instantiation of theModel
class within the method has been modified accordingly. This change is consistent with the renaming effort described in the PR.examples/e2e/steps/promotion/promote_with_metric_compare.py (3)
20-20: The import statement has been updated to use the
Model
class instead ofModelVersion
, aligning with the renaming operation.61-62: The
model_version
object has been replaced with themodel
object, which is consistent with the renaming fromModelVersion
toModel
.85-86: The
set_stage
method is now called on themodel
object instead ofmodel_version
, which is in line with the renaming effort.src/zenml/pipelines/pipeline_decorator.py (3)
29-29: The import statement for the
Model
class has been updated, replacing the previousModelVersion
import, which is consistent with the renaming operation.71-71: The
model_version
parameter has been renamed tomodel
in thepipeline
function, aligning with the renaming fromModelVersion
toModel
.88-88: The
model_version
parameter has been renamed tomodel
in the inner decorator of thepipeline
function, which is consistent with the renaming effort.src/zenml/artifacts/external_artifact.py (2)
59-60: The parameter
model_version
has been renamed tomodel
, and the parameterversion
now refers to the artifact version, whilemodel
refers to the model version. This change is consistent with the renaming operation and clarifies the distinction between artifact version and model version.152-152: The
model
attribute is correctly used in theconfig
method of theExternalArtifact
class, aligning with the renaming fromModelVersion
toModel
.src/zenml/new/steps/step_decorator.py (3)
36-36: The import statement for the
Model
class has been updated, replacing the previousModelVersion
import, which is consistent with the renaming operation.70-70: The
model_version
parameter has been replaced with themodel
parameter in thestep
function, aligning with the renaming fromModelVersion
toModel
.90-90: The
model_version
parameter has been replaced with themodel
parameter in the inner decorator of thestep
function, which is consistent with the renaming effort.src/zenml/model/utils.py (5)
29-29: The import statement for the
Model
class has been updated, replacing the previousModelVersion
import, which is consistent with the renaming operation.55-57: The variable
model_version
has been renamed tomodel
, and the check for the model context has been updated accordingly. This change aligns with the PR's objective of renamingModelVersion
toModel
across the codebase.66-70: The implicit linking of artifacts to the model has been updated to use the
model
variable instead ofmodel_version
. The log message has been adjusted to reflect the new naming convention.77-106: The function
link_artifact_config_to_model_version
has been updated to accept amodel
parameter instead ofmodel_version
. The logic within the function has been adjusted to work with theModel
class, which is consistent with the renaming effort.178-185: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [171-183]
The function
log_model_metadata
has been updated to work with theModel
class instead ofModelVersion
. The logic within the function has been adjusted to reflect the new naming convention.docs/book/user-guide/starter-guide/structuring-a-project.md (2)
80-83: In the
predict
function, themodel_version
variable has been replaced withmodel
, and theget_model_artifact
method is now called on themodel
object. This change aligns with the PR's objective of renamingModelVersion
toModel
across the codebase.110-122: In the
do_predictions
pipeline, themodel_config
parameter has been updated to use theModel
class with theModelStages.PRODUCTION
version. Themodel
object is then used to get the model artifact. This change is consistent with the renaming effort and the usage of theModel
class in the pipeline configuration.src/zenml/steps/step_decorator.py (4)
39-39: The import statement for the
Model
class has been updated, replacing the previousModelVersion
import, which is consistent with the renaming operation.111-111: The
model_version
parameter has been renamed tomodel
in thestep
function, aligning with the renaming fromModelVersion
toModel
.131-131: The
model_version
parameter has been renamed tomodel
in the inner decorator of thestep
function, which is consistent with the renaming effort.164-164: The
model
parameter is correctly used in thestep
function, aligning with the renaming fromModelVersion
toModel
.src/zenml/config/step_configurations.py (1)
- 137-137: The
model_version
attribute has been renamed tomodel
in theStepConfigurationUpdate
class, which is consistent with the renaming operation fromModelVersion
toModel
.examples/quickstart/run.py (2)
232-232: The code correctly updates the dictionary keys to reflect the new naming convention from
model_version
tomodel
.232-232: Ensure that the
config
dictionary is populated with the correct keys and values elsewhere in the codebase to match this new convention.src/zenml/models/v2/core/model.py (2)
34-34: The import statement has been correctly updated to reflect the new naming convention, importing
Model
instead ofModelVersion
.310-316: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [313-326]
The
versions
property method has been updated to return a list ofModel
objects. Ensure that theto_model_class
method and thelist_model_versions
function are correctly implemented to handle the newModel
class..github/workflows/update-templates-to-examples.yml (2)
53-53: The
ref-template
value has been correctly updated to "2024.01.12" to align withZENML_PROJECT_TEMPLATES
fromsrc/zenml/cli/base.py
.53-53: Ensure that the
ZENML_PROJECT_TEMPLATES
variable insrc/zenml/cli/base.py
is indeed set to the date "2024.01.12".Verification successful
The
ZENML_PROJECT_TEMPLATES
variable insrc/zenml/cli/base.py
is indeed set to the date "2024.01.12" for thee2e_batch
andstarter
templates, as required by the review comment. This confirms that theref-template
value in the.github/workflows/update-templates-to-examples.yml
file is aligned with theZENML_PROJECT_TEMPLATES
fromsrc/zenml/cli/base.py
.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that 'ZENML_PROJECT_TEMPLATES' in 'src/zenml/cli/base.py' is set to "2024.01.12". rg --type python 'ZENML_PROJECT_TEMPLATES' 'src/zenml/cli/base.py'Length of output: 95
Script:
#!/bin/bash # Verify that 'ZENML_PROJECT_TEMPLATES' in 'src/zenml/cli/base.py' is set to "2024.01.12". rg 'ZENML_PROJECT_TEMPLATES' src/zenml/cli/base.pyLength of output: 304
Script:
#!/bin/bash # Extract the assignment of 'ZENML_PROJECT_TEMPLATES' to confirm the value "2024.01.12". rg 'ZENML_PROJECT_TEMPLATES = ' src/zenml/cli/base.py -A 10Length of output: 605
src/zenml/pipelines/base_pipeline.py (1)
- 52-52: The renaming of
PARAM_MODEL_VERSION
toPARAM_MODEL
is consistent with the PR's objective of renamingModelVersion
toModel
.tests/integration/functional/pipelines/test_pipeline_config.py (3)
23-40: The test function
assert_model_step
has been updated to reflect the newModel
entity and its properties. The assertions are correctly checking the properties of theModel
object.65-80: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [52-77]
The test
test_pipeline_with_model_from_yaml
correctly creates aModel
object and uses it in a pipeline configuration. The test also ensures that theModel
object's properties are correctly serialized and deserialized from a YAML configuration file.
- 103-135: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [106-158]
The test
test_pipeline_config_from_file_not_overridden_for_model
correctly updates theModel
object and its properties. It also checks that the configuration from the YAML file is not overridden when theconfigure
method is called with a newModel
object.docs/book/user-guide/starter-guide/track-ml-models.md (4)
- 58-69: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [45-77]
The documentation has been updated to use the
Model
entity instead ofModelVersion
. The example code and descriptions are consistent with the new naming convention.
- 128-138: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [131-144]
The documentation correctly demonstrates how to fetch a
Model
object within a pipeline using theget_step_context
andget_pipeline_context
functions.
- 170-170: The
log_model_metadata
function is used appropriately to log metadata to theModel
object. Ensure that this function is implemented and handles theModel
class correctly.Verification successful
The
log_model_metadata
function is indeed implemented within thesrc/zenml/model/utils.py
file. This confirms that the function exists and is part of the codebase, which aligns with the usage mentioned in the review comment.* 224-246: The documentation provides clear examples of how to interact with the `Model` entity, including fetching the latest model version, getting a model by version, and setting the stage of a model.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify the implementation of 'log_model_metadata' function for the 'Model' class. ast-grep --lang python --pattern $'def log_model_metadata($_, $_, $_) { $$$ }'Length of output: 544
Script:
#!/bin/bash # Correcting the syntax and trying to verify the implementation of 'log_model_metadata' function for the 'Model' class. ast-grep --lang python --pattern $'def log_model_metadata($$$) { $$$ }'Length of output: 530
Script:
#!/bin/bash # Simplifying the syntax and trying again to verify the implementation of 'log_model_metadata' function for the 'Model' class. ast-grep --lang python --pattern $'def log_model_metadata($$$)'Length of output: 409
Script:
#!/bin/bash # Using ripgrep to search for the 'log_model_metadata' function within Python files. rg "def log_model_metadata" -t pyLength of output: 84
docs/book/user-guide/advanced-guide/data-management/model-management.md (6)
68-68: The Python SDK example correctly uses the new
Model
class for model registration. The code snippet is syntactically correct and aligns with the renaming objective.82-83: The implicit registration example correctly uses the
Model
object within the@pipeline
decorator. This change is consistent with the renaming effort and the example is clear and informative.129-135: The explicit version naming example has been updated to use the
Model
class with aversion
argument. This is a good example of how to explicitly set a version for a model, and the code snippet is correct.154-173: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [157-186]
The autonumbering of versions section correctly demonstrates how ZenML handles version numbering when not specified by the user. The example is updated to use the
Model
class and is syntactically correct.
238-244: The promotion via Python SDK section correctly uses the
Model
class to promote a model to a specific stage. The code snippet is correct and demonstrates the intended functionality.275-285: The artifact configuration section correctly uses the
Model
class in the@pipeline
decorator to link artifacts to a model. The code snippet is correct and aligns with the renaming effort.tests/integration/functional/artifacts/test_artifact_config.py (9)
22-22: The import statement correctly reflects the renaming from
ModelVersion
toModel
. This change is consistent with the PR's objective.27-41: The
@step
decorators have been updated to use theModel
class instead ofModelVersion
. The changes are consistent and correctly reflect the renaming effort.96-96: The
multi_named_output_step_from_context
function correctly uses theModel
class in the@step
decorator. The test function name and comments are consistent with the code changes.132-140: The
multi_named_output_step_not_tracked
function correctly uses theModel
class in the@step
decorator. The comments have been updated to reflect the change fromModelVersion
toModel
.247-247: The
multi_named_output_step_mixed_linkage
function correctly uses theModel
class with explicit version naming. The test function name and comments are consistent with the code changes.282-282: The
and_some_typed_outputs
function correctly uses theModel
class in the@step
decorator. The comments have been updated to reflect the change fromModelVersion
toModel
.290-290: The
multi_named_pipeline_mixed_linkage
pipeline definition correctly uses theModel
class in the@pipeline
decorator. The comments have been updated to reflect the change fromModelVersion
toModel
.372-372: The
_inner_pipeline
pipeline definition correctly uses theModel
class in the@pipeline
decorator. The comments have been updated to reflect the change fromModelVersion
toModel
.422-422: The
_inner_pipeline
pipeline definition correctly uses theModel
class in the@pipeline
decorator. The comments have been updated to reflect the change fromModelVersion
toModel
.src/zenml/new/steps/step_context.py (2)
35-35: The import statement correctly reflects the renaming from
ModelVersion
toModel
. This change is consistent with the PR's objective.166-192: The
model
property has been updated to return aModel
object instead of aModelVersion
. The logic for resolving the model from the step or pipeline configuration is correct and the error messages have been updated accordingly.tests/integration/functional/model/test_model_version.py (20)
22-22: The import statement correctly reflects the renaming from
ModelVersion
toModel
. This change is consistent with the PR's objective.66-68: The
step_metadata_logging_functional
function correctly uses theModel
class to log metadata. The test function name and comments are consistent with the code changes.83-84: The
consume_from_model_version
function correctly uses theModel
class to load an artifact from a specific model version. The test function name and comments are consistent with the code changes.98-99: The
test_model_created_with_warning
test function correctly uses theModel
class to create a model and checks for the appropriate logging behavior. The test function name and comments are consistent with the code changes.107-108: The
test_model_exists
test function correctly uses theModel
class to fetch an existing model and checks for the absence of a warning, which is the expected behavior. The test function name and comments are consistent with the code changes.117-118: The
test_model_create_model_and_version
test function correctly uses theModel
class to create a model and a model version, and checks for the appropriate logging behavior. The test function name and comments are consistent with the code changes.131-138: The
test_create_model_version_makes_proper_tagging
test function correctly uses theModel
class to create a model version with tags and checks that the tags are unique. The test function name and comments are consistent with the code changes.150-151: The
test_model_fetch_model_and_version_by_number
test function correctly uses theModel
class to retrieve a model and a model version by exact version number. The test function name and comments are consistent with the code changes.173-174: The
test_model_fetch_model_and_version_by_stage
test function correctly uses theModel
class to retrieve a model and a model version by exact stage number. The test function name and comments are consistent with the code changes.201-202: The
test_init_stage_logic
test function correctly uses theModel
class to inform the user about the version being set to a string contained inModelStages
. The test function name and comments are consistent with the code changes.215-219: The
test_recovery_flow
test function correctly uses theModel
class to recover the same version after a failure. The test function name and comments are consistent with the code changes.227-227: The
test_tags_properly_created
test function correctly uses theModel
class to create proper tag relationships. The test function name and comments are consistent with the code changes.244-244: The
test_tags_properly_updated
test function correctly uses theModel
class to update tag relationships. The test function name and comments are consistent with the code changes.286-292: The
test_model_config_differs_from_db_warns
test function correctly uses theModel
class to warn if the model config differs from the database. The test function name and comments are consistent with the code changes.310-317: The
test_model_version_config_differs_from_db_warns
test function correctly uses theModel
class to warn if the model version config differs from the database. The test function name and comments are consistent with the code changes.337-337: The
test_metadata_logging
test function correctly uses theModel
class to track metadata from an object. The test function name and comments are consistent with the code changes.354-354: The
test_metadata_logging_functional
test function correctly uses theModel
class to track metadata from a function. The test function name and comments are consistent with the code changes.382-382: The
test_metadata_logging_in_steps
test function correctly uses theModel
class to track metadata from a function in steps. The test function name and comments are consistent with the code changes.400-403: The
test_deletion_of_links
test function correctly uses theModel
class to delete artifact links (with artifacts) from a model. The test function name and comments are consistent with the code changes.464-464: The
test_that_artifacts_are_not_linked_to_models_outside_of_the_context
test function correctly uses theModel
class to ensure artifacts are linked only to model versions from the context. The test function name and comments are consistent with the code changes.src/zenml/models/v2/core/model_version.py (2)
35-35: The import statement has been updated to reflect the renaming from
ModelVersion
toModel
. This change is consistent with the PR objectives.311-322: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [314-332]
The method
to_model_class
has been renamed fromto_model_version
and the return type has been updated accordingly. This change is in line with the renaming effort described in the PR objectives. Ensure that all references to this method throughout the codebase are updated to use the new method name and handle the new return type correctly.src/zenml/orchestrators/step_launcher.py (2)
56-56: The import statement has been updated to reflect the renaming from
ModelVersion
toModel
. This change is consistent with the PR objectives.313-315: The variable
model
is being assigned a value from the step configuration or pipeline configuration. This change is part of the renaming effort and should be checked for consistency across the codebase.Verification successful
The shell script results indicate that the pattern
$_ = $_.pipeline_configuration.model
is present in the filepipeline.py
at line 927, wherepipeline_model
is being assigned frompipeline_configuration.model
. However, there were no matches for the pattern involvingstep_configurations
, either with or without theor
condition.This suggests that the renaming from
ModelVersion
toModel
has been partially applied in the codebase. The assignment frompipeline_configuration
to amodel
variable is consistent with the change observed instep_launcher.py
, but the absence of similar patterns forstep_configurations
could indicate an incomplete renaming effort or that the pattern does not exist in that specific form elsewhere in the codebase.Given this information, the review comment about the variable
model
being assigned from step or pipeline configuration is verified in the context ofpipeline_configuration.model
, but there is no evidence of a similar pattern forstep_configurations
. This could mean that the renaming is consistent for thepipeline_configuration
part but cannot be confirmed forstep_configurations
due to the lack of matches.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search to find files that assign `model` from step or pipeline configuration. Expecting list of locations. ast-grep --lang python --pattern $'$_ = $_.step_configurations[$_].config.model or $_.pipeline_configuration.model'Length of output: 116
Script:
#!/bin/bash # Broaden the search to find any assignments of `model` that might be related to step or pipeline configurations. ast-grep --lang python --pattern $'$_ = $_.step_configurations[$$$].config.model or $_.pipeline_configuration.model' # Additionally, search for any assignments of `model` from `step_configurations` or `pipeline_configuration` without the `or` condition. ast-grep --lang python --pattern $'$_ = $_.step_configurations[$$$].config.model' ast-grep --lang python --pattern $'$_ = $_.pipeline_configuration.model'Length of output: 385
src/zenml/cli/model.py (1)
- 63-63: The method call
to_model_class
has been updated fromto_model_version
. This change is consistent with the renaming effort and should be reflected in all CLI commands that interact with models.src/zenml/config/compiler.py (1)
- 207-207: The parameter
model_version
has been renamed tomodel
in the_apply_run_configuration
function. Ensure that all references to this parameter throughout the codebase have been updated to reflect this change.src/zenml/model/model.py (7)
45-46: The class name change from
ModelVersion
toModel
is consistent with the PR's objective of renaming throughout the codebase.14-14: The module docstring succinctly describes the purpose of the file, which is good for maintainability.
196-196: The method
_try_get_as_external_artifact
correctly creates anExternalArtifact
instance with the updatedmodel
attribute. This change aligns with the renaming objective.637-638: The method
_get_or_create_model_version
is called within themodel
context. This is a critical part of the renaming and should be tested to ensure that the method behaves as expected with the new class name.430-430: The
__eq__
method has been updated to compare instances ofModel
instead ofModelVersion
. This is a logical change given the renaming. However, ensure that this method is tested for correctness.Verification successful
The output from the ripgrep command provides the implementation of the
__eq__
method within theModel
class in thesrc/zenml/model/model.py
file. The method includes a type check to ensure that theother
object is an instance ofModel
and returnsNotImplemented
if it is not. However, the output does not show the complete logic of the__eq__
method beyond the type check.To fully verify the correctness of the
__eq__
method, we need to see the entire method body. Since the output is truncated, I will need to run another script to capture the rest of the method.
The extended output from the ripgrep command provides the full implementation of the
__eq__
method for theModel
class insrc/zenml/model/model.py
. The method performs the following checks:
- It checks if the
other
object is an instance ofModel
, returningNotImplemented
if it is not.- It compares the
name
attributes ofself
andother
, returningFalse
if they are not equal.- It checks if both the
name
andversion
attributes are equal, returningTrue
if they are.- It retrieves or creates a model version for
self
andother
and compares theirid
attributes, returningTrue
if they are equal.This implementation suggests that two
Model
instances are considered equal if they have the samename
andversion
, or if their model versionid
s are equal. This logic appears to be consistent with the intent of an equality check within the context of model comparison.Based on the provided output, the
__eq__
method seems to be correctly implemented following the renaming fromModelVersion
toModel
. However, the comment within the method still references "ModelVersions" which should be updated to reflect the new class nameModel
.* 693-703: The `_merge` method has been updated to merge attributes from another `Model` instance. This method should be reviewed to ensure that it correctly handles merging of all relevant attributes and that it is used appropriately in the codebase.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that the `__eq__` method is correctly implemented for the `Model` class. ast-grep --lang python --pattern $'class Model { $$$ def __eq__($_, $_) { $$$ } $$$ }'Length of output: 446
Script:
#!/bin/bash # Refine the ast-grep pattern and use ripgrep to confirm the existence and implementation of the `__eq__` method in the `Model` class. ast-grep --lang python --pattern 'class Model { def __eq__($$$) { $$$ } }' rg '__eq__' -A 3 -B 3 --glob '*model.py'Length of output: 775
Script:
#!/bin/bash # Use ripgrep to capture the full implementation of the `__eq__` method in the `Model` class. rg '__eq__' -A 10 -B 2 --glob '*model.py'Length of output: 695
Script:
#!/bin/bash # Extend the ripgrep search to capture the entire `__eq__` method implementation. rg '__eq__' -A 20 -B 2 --glob '*model.py'Length of output: 1354
- 707-707: The
__hash__
method has been updated to generate a hash for theModel
instance. This change is consistent with the renaming and should be tested to ensure that it generates a unique and consistent hash value.src/zenml/orchestrators/step_runner.py (3)
637-638: The method
_prepare_model_context_for_step
has been updated to use the newmodel
context. This change is consistent with the renaming objective and should be tested to ensure that the method behaves as expected.683-683: The method
_get_model_versions_from_config
has been updated to use the newmodel
context. This change is consistent with the renaming objective and should be tested to ensure that the method behaves as expected.734-738: The method
_link_pipeline_run_to_model_from_artifacts
has been updated to use the newmodel
context. This change is consistent with the renaming objective and should be tested to ensure that the method behaves as expected.examples/quickstart/quickstart.ipynb (8)
145-145: The import statement correctly reflects the renaming from
ModelVersion
toModel
. This change is consistent with the PR objectives.942-946: The
inference_predict
function has been updated to use theModel
object instead ofModelVersion
. Ensure that theModel
class has a methodload_artifact
that can be used in this context.Verification successful
The search using
rg
has identified that aload_artifact
method does exist within themodel.py
file under theModel
class. This confirms that theModel
class has theload_artifact
method which is used in theinference_predict
function, as mentioned in the review comment.* 732-732: The markdown documentation correctly refers to the `Model` object, which is in line with the renaming operation.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that the Model class has a load_artifact method. ast-grep --lang python --pattern $'class Model { $$$ load_artifact($_) { $$$ } $$$ }'Length of output: 445
Script:
#!/bin/bash # Try a more general search for the load_artifact method within the Model class. ast-grep --lang python --pattern $'class Model { $$$ def load_artifact($$$) { $$$ } $$$ }'Length of output: 450
Script:
#!/bin/bash # Perform a broader search for any occurrence of load_artifact in the codebase. rg "def load_artifact"Length of output: 343
745-745: The creation of a
Model
object with metadata is consistent with the renaming fromModelVersion
toModel
. This change is correctly reflected in the documentation.761-761: The setting of the model version to "sgd" is appropriate and follows the new naming convention.
783-783: The setting of the model version to "rf" is consistent with the renaming and the intended use of the
Model
object.997-997: The markdown explanation correctly describes the use of the
Model
object with theproduction
stage, which aligns with the renaming operation.1011-1011: The configuration of the
Model
object with theproduction
stage is correct and reflects the changes made in the codebase.tests/integration/functional/zen_stores/utils.py (2)
- 618-618: The class
ModelVersionContext
has been renamed toModelContext
. This change is consistent with the PR's objective of renamingModelVersion
toModel
. Ensure that all references to this class throughout the codebase have been updated to reflect the new name.Verification successful
Based on the output from the shell scripts, it appears that the class
ModelVersionContext
has been successfully renamed toModelContext
and the new class name is being used throughout the codebase. There are no remaining references to the old class name, and the new class name is present in multiple test files, which suggests that the renaming has been applied consistently.However, to ensure thoroughness, it would be prudent to verify that the file
test_model_version.py
has been updated correctly in the context of the renaming, given its name suggests a close relationship with the old class name.
The output from the shell script confirms that the file
test_model_version.py
has been updated to use the newModelContext
class name. The references toModelContext
within the file are consistent with the expected usage of the renamed class. There are no instances of the oldModelVersionContext
class name, which aligns with the PR's objective of renamingModelVersion
toModel
.Given this information, it can be concluded that the renaming has been applied correctly and consistently within the
test_model_version.py
file as well.* 615-621: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [1-617]Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search to find files that import ModelVersionContext. Expecting no results. rg --type py 'ModelVersionContext' # Search new class name to validate usage. Expecting list of locations where the new class name is used. rg --type py 'ModelContext'Length of output: 5521
Script:
#!/bin/bash # Verify that the file test_model_version.py has been updated correctly in the context of the renaming. rg --type py 'ModelContext' tests/integration/functional/model/test_model_version.pyLength of output: 653
Ensure that the renaming of
ModelVersionContext
toModelContext
has been propagated throughout the entire file. Any methods, comments, or documentation that reference the old class name should be updated to maintain consistency.Verification successful
The search for references to
ModelVersionContext
in the filetests/integration/functional/zen_stores/utils.py
did not yield any results. This suggests that there are no leftover references toModelVersionContext
in this file, and it is likely that the renaming toModelContext
has been properly propagated throughout the file.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search to find any leftover references to ModelVersionContext in the file. rg --type py 'ModelVersionContext' tests/integration/functional/zen_stores/utils.pyLength of output: 83
src/zenml/steps/base_step.py (8)
73-73: Renaming import from
ModelVersion
toModel
is consistent with the PR objectives.139-139: The addition of the
model
parameter in the__init__
method is consistent with the renaming effort.215-221: Logging the model context with its name and version is a good practice for debugging and traceability.
241-241: Passing the
model
parameter to theconfigure
method aligns with the changes and ensures that the model configuration is applied to the step.656-656: The
configure
method has been updated to include themodel
parameter, which is consistent with the changes made in the__init__
method.791-791: The
with_options
method has been updated to include themodel
parameter, ensuring that the step can be configured with model options.768-768: The
model
parameter is correctly included in the step configuration update within theconfigure
method.842-842: The
model
parameter is correctly passed to theconfigure
method withinwith_options
, ensuring that the copied step includes the model configuration.tests/integration/functional/test_client.py (13)
50-50: The import statement for
Model
has been updated to reflect the renaming fromModelVersion
toModel
. This change is consistent with the PR objectives.1582-1582: The variable
mv2
is assigned the result ofto_model_class
which seems to be a method that was previously associated withModelVersion
. The usage here implies that the method is now part of theModel
class. This needs to be verified to ensure that the methodto_model_class
is correctly implemented in theModel
class and that its usage here is valid.1588-1588: The creation of a new model version using
create_model_version
followed byto_model_class
is consistent with the renaming operation. The methodto_model_class
is used again, which needs to be verified as mentioned in the previous comment.1592-1592: The variable
mv4
is assigned similarly tomv2
, using theto_model_class
method. The same verification applies here to ensure consistency and correctness.1610-1610: The retrieval of a model by stage using
get_model_version
and converting it to a class withto_model_class
is consistent with the new naming convention. The usage ofto_model_class
needs to be verified as before.1582-1582: The usage of
to_model_class
within theget_model_version
call is potentially incorrect due to the renaming. This should be verified to ensure that the method exists on theModel
class and is being used correctly.1588-1588: Again, the usage of
to_model_class
is noted. It is important to verify that this method is appropriate for theModel
class.1592-1592: The
to_model_class
method is used once more. Consistency with the previous usages is good, but verification is still needed.1610-1610: The
to_model_class
method is used again in the context of retrieving a model by stage. This usage pattern is consistent throughout the file, but the method's existence and correctness must be verified.1582-1582: The
to_model_class
method is used after getting the model version. This needs to be verified for correctness.1588-1588: The creation of a new model version and subsequent conversion to a model class is consistent with the renaming. The
to_model_class
method's usage should be verified.1592-1592: The
to_model_class
method is used again. The same verification applies here to ensure consistency and correctness.1610-1610: The retrieval of a model by stage using
get_model_version
and converting it to a class withto_model_class
is consistent with the new naming convention. The usage ofto_model_class
needs to be verified as before.src/zenml/new/pipelines/pipeline.py (9)
73-73: The import statement for
NewModelRequest
has been updated to reflect the renaming fromModelVersion
toModel
. This change is consistent with the PR objectives.95-95: The import statement for
Model
has been added. This is a new import and is part of the renaming effort. Ensure that theModel
class is correctly defined elsewhere in the codebase.Verification successful
The
Model
class is correctly defined within the codebase, as confirmed by the output from the executed shell script. The renaming effort fromModelVersion
toModel
appears to be properly implemented in this aspect.* 149-149: The docstring for the `model` parameter has been updated to reflect the new terminology. It's important to ensure that all references to `ModelVersion` are updated to `Model` in the documentation as well.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that the Model class exists and is correctly defined. fd 'model.py' --exec cat {} \; | rg 'class Model'Length of output: 329
168-168: The
model
parameter is being passed to theconfigure
method within thePipeline
class constructor. This is part of the renaming changes and should be consistent throughout the codebase.307-307: The
model
parameter has been added to theconfigure
method signature. This change is consistent with the renaming strategy and should be checked for consistency across the codebase.Verification successful
The extracted method signatures from both
base_step.py
andpipeline.py
show that themodel
parameter has been added to theconfigure
method in both cases. This parameter is optional and has a default value ofNone
. The addition of themodel
parameter is consistent with the renaming strategy fromModelVersion
toModel
as described in the PR objectives.Based on the information provided, it appears that the
model
parameter has been integrated consistently in theconfigure
method across the two identified files in the codebase.* 343-343: The docstring for the `model` parameter in the `configure` method has been updated to reflect the new terminology. It's important to ensure that all references to `ModelVersion` are updated to `Model` in the documentation as well.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that the `model` parameter is being used consistently in the `configure` method calls. rg --type python 'configure\('Length of output: 63
Script:
#!/bin/bash # Correcting the type flag and verifying that the `model` parameter is being used consistently in the `configure` method calls. rg --type py 'def configure\('Length of output: 135
Script:
#!/bin/bash # Extract the method signatures of the `configure` method from the identified files. sed -n '/def configure(/,/):/p' src/zenml/steps/base_step.py sed -n '/def configure(/,/):/p' src/zenml/new/pipelines/pipeline.pyLength of output: 7568
369-369: The
model
parameter is being included in the dictionary of values within theconfigure
method. This is part of the renaming changes and should be consistent throughout the codebase.874-874: The
model
parameter has been added to the_update_new_requesters
method. This change is consistent with the renaming strategy and should be checked for consistency across the codebase.Verification successful
The output from the script provides the context around the definition and calls to the
_update_new_requesters
method within thepipeline.py
file. From the output, we can observe the following:
- The method
_update_new_requesters
is defined with themodel
parameter of type"Model"
.- There are two calls to
_update_new_requesters
within the same file:
- The first call passes
model=step_model
, along with other parameters.- The second call passes
model=pipeline_model
, along with other parameters.Both calls to
_update_new_requesters
are consistent with the method definition, using themodel
parameter as part of the method signature. This confirms that themodel
parameter is being used consistently in the_update_new_requesters
method calls within thepipeline.py
file.* 1393-1402: The parsing of the `model` configuration from a file has been updated to reflect the new terminology. This change is consistent with the renaming strategy and should be checked for consistency across the codebase.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that the `model` parameter is being used consistently in the `_update_new_requesters` method calls. rg --type python '_update_new_requesters\('Length of output: 76
Script:
#!/bin/bash # Verify that the `model` parameter is being used consistently in the `_update_new_requesters` method calls. rg '_update_new_requesters\(' --type pyLength of output: 271
Script:
#!/bin/bash # Extract the method definition and calls to `_update_new_requesters` to verify the `model` parameter usage. rg --context 5 '_update_new_requesters\(' src/zenml/new/pipelines/pipeline.pyLength of output: 1365
Verification successful
The search results indicate that the
Model.parse_obj()
method is indeed used in thepipeline.py
file as part of the changes made in the PR. Additionally, the method is used in various other files within thezenml
directory, such asrest_zen_store.py
,auth.py
, and_hub/client.py
. These instances suggest that parsing themodel
configuration from a file is a pattern used consistently across the codebase.Based on the information provided, the parsing of the
model
configuration usingModel.parse_obj()
inpipeline.py
is consistent with other parts of the codebase where model configurations are parsed from files. This supports the review comment that the change is consistent with the renaming strategy and should be checked for consistency across the codebase.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that the `model` configuration is being parsed correctly from files elsewhere in the codebase. rg --type python 'parse_obj\('Length of output: 63
Script:
#!/bin/bash # Verify that the `model` configuration is being parsed correctly from files elsewhere in the codebase. # Correcting the file type to 'py' and refining the search pattern. rg --type py 'Model\.parse_obj\('Length of output: 1231
tests/integration/functional/zen_stores/test_zen_store.py (42)
31-31: Renaming
ModelVersionContext
toModelContext
is consistent with the PR's objectives. Ensure that all references to the old context are updated across the entire test suite.542-553: The renaming of the context variable from
model_version_context
tomodel_context
and the subsequent call tomodel_context.cleanup()
are correctly implemented. However, ensure that the cleanup method is correctly defined in theModelContext
class and that it performs the expected cleanup operations.889-900: Similar to the previous comment, the renaming and cleanup call are correct. It's important to verify that the
ModelContext
is handling the cleanup for service accounts as expected.3611-3611: The use of
ModelContext
in the test casetest_latest_version_properly_fetched
is appropriate. Confirm that theModelContext
is correctly implemented to fetch the latest version.3635-3635: The test case
test_update_name
usesModelContext
correctly. Ensure that the renaming does not affect the logic of updating the model's name.3653-3653: The test case
test_create_pass
has been updated to useModelContext
. Verify that thecreate_model_version
method andModelVersionRequest
are still relevant and correctly renamed if necessary.3666-3666: The test case
test_create_duplicated
also usesModelContext
. Ensure that the logic for handling duplicated creations is still valid after the renaming.3688-3688: In the test case
test_create_no_model
, confirm that theModelContext
and the related exception handling are correctly implemented to test the database relations.3702-3702: The test case
test_get_not_found
usesModelContext
. Verify that theget_model_version
method is correctly handling the case where a model version is not found.3711-3711: The test case
test_get_found
correctly usesModelContext
. Confirm that thecreate_model_version
and retrieval logic are consistent with the new naming convention.3731-3731: The test case
test_list_empty
usesModelContext
. Ensure that thelist_model_versions
method is correctly implemented to handle an empty list of model versions.3741-3741: The test case
test_list_not_empty
usesModelContext
. Confirm that thecreate_model_version
method and the listing logic are still valid with the new naming.3769-3769: In the test case
test_delete_not_found
, ensure thatModelContext
and thedelete_model_version
method are correctly handling the case where a model version to be deleted is not found.3778-3778: The test case
test_delete_found
usesModelContext
. Verify that the creation and deletion logic for model versions are consistent with the new naming convention.3801-3801: The test case
test_update_not_found
usesModelContext
. Ensure that the update logic and exception handling are correct when a model version is not found.3815-3815: In the test case
test_update_not_forced
, confirm thatModelContext
is correctly used and that the update logic is valid for unforced updates on existing stage versions.3856-3856: The test case
test_update_name_and_description
usesModelContext
. Verify that the update logic for name and description is still accurate after the renaming.3885-3885: In the test case
test_in_stage_not_found
, ensure thatModelContext
is correctly used and that the logic for handling the case where a model version in a stage is not found is still valid.3907-3907: The test case
test_latest_found
usesModelContext
. Confirm that the logic for retrieving the latest model version is consistent with the new naming convention.3933-3933: In the test case
test_update_forced
, ensure thatModelContext
is correctly used and that the update logic is valid when the model version in a stage exists andforce=True
.3996-3996: The test case
test_update_public_interface
usesModelContext
. Verify that the update logic through the public interface is still accurate after the renaming.4024-4024: In the test case
test_update_public_interface_bad_stage
, confirm thatModelContext
is correctly used and that the logic for handling bad stage values is still valid.4050-4050: The test case
test_increments_version_number
usesModelContext
. Ensure that the logic for incrementing version numbers on sequential insertions is consistent with the new naming convention.4082-4082: In the test case
test_get_found_by_number
, confirm thatModelContext
is correctly used and that the logic for retrieving model versions by integer version number is still valid.4094-4094: The test case
test_get_not_found_by_number
usesModelContext
. Verify that the logic for handling not found cases by integer version number and string version number is still accurate.4107-4107: The test case
test_link_create_pass
withinTestModelVersionArtifactLinks
usesModelContext
. Confirm that the context is correctly implemented to handle artifact links.4123-4123: In the test case
test_link_create_versioned
, ensure thatModelContext
is correctly used and that the logic for creating versioned artifact links is still valid.4151-4151: The test case
test_link_create_duplicated_by_id
usesModelContext
. Verify that the logic for handling duplicated artifact links by ID is consistent with the new naming convention.4181-4181: In the test case
test_link_create_single_version_of_same_output_name_from_different_steps
, confirm thatModelContext
is correctly used and that the logic for handling links from different steps is still valid.4213-4213: The test case
test_link_delete_found
withinTestModelVersionArtifactLinks
usesModelContext
. Ensure that the deletion logic for artifact links is consistent with the new naming convention.4239-4239: In the test case
test_link_delete_all
, confirm thatModelContext
is correctly used and that the logic for deleting all artifact links is still valid.4265-4265: The test case
test_link_delete_not_found
usesModelContext
. Verify that the exception handling for not found cases during artifact link deletion is accurate.4274-4274: In the test case
test_link_list_empty
, ensure thatModelContext
is correctly used and that the logic for listing artifact links when empty is still valid.4284-4284: The test case
test_link_list_populated
usesModelContext
. Confirm that the logic for listing populated artifact links is consistent with the new naming convention.4378-4378: The test case
test_link_create_pass
withinTestModelVersionPipelineRunLinks
usesModelContext
. Ensure that the context is correctly implemented to handle pipeline run links.4395-4395: In the test case
test_link_create_duplicated
, confirm thatModelContext
is correctly used and that the logic for handling duplicated pipeline run links is still valid.4421-4421: The test case
test_link_delete_found
usesModelContext
. Verify that the deletion logic for pipeline run links is consistent with the new naming convention.4448-4448: In the test case
test_link_delete_not_found
, ensure thatModelContext
is correctly used and that the exception handling for not found cases during pipeline run link deletion is accurate.4456-4456: The test case
test_link_list_empty
usesModelContext
. Confirm that the logic for listing pipeline run links when empty is still valid.4466-4466: In the test case
test_link_list_populated
, ensure thatModelContext
is correctly used and that the logic for listing populated pipeline run links is consistent with the new naming convention.4694-4694: The test case related to tag deletion uses
ModelContext
. Verify that the logic for deleting a link on tag deletion is still accurate after the renaming.4778-4781: The code snippet seems to be part of a larger context-switching block. Ensure that the
Model
class and the_get_or_create_model_version
method are correctly implemented and renamed if necessary.
NLP template updates in |
docs/book/user-guide/advanced-guide/data-management/model-management.md
Outdated
Show resolved
Hide resolved
docs/book/user-guide/advanced-guide/data-management/model-management.md
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah big PR, lots of changes, hard to review. The key thing is that tests are passing. It looks good to me, aside from the comments I already made but they might be unresolvable.
@strickvl I gave this PR a second fresh look and tried to rename all confusing statements I was able to find. Sure it's not perfect, as this is huge renaming. |
@coderabbitai review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 0
Configuration used: .coderabbit.yaml
Files selected for processing (27)
- .github/workflows/update-templates-to-examples.yml (7 hunks)
- docs/book/getting-started/core-concepts.md (1 hunks)
- docs/book/user-guide/advanced-guide/data-management/logging-metadata.md (5 hunks)
- docs/book/user-guide/advanced-guide/data-management/model-management.md (11 hunks)
- docs/book/user-guide/advanced-guide/data-management/tagging.md (1 hunks)
- docs/book/user-guide/advanced-guide/pipelining-features/fetch-metadata-within-steps.md (2 hunks)
- docs/book/user-guide/starter-guide/structuring-a-project.md (4 hunks)
- docs/book/user-guide/starter-guide/track-ml-models.md (11 hunks)
- examples/e2e_nlp/.copier-answers.yml (1 hunks)
- examples/e2e_nlp/run.py (4 hunks)
- examples/e2e_nlp/steps/deploying/save_model.py (1 hunks)
- examples/e2e_nlp/steps/promotion/promote_get_metrics.py (2 hunks)
- examples/e2e_nlp/steps/promotion/promote_metric_compare_promoter.py (1 hunks)
- src/zenml/artifacts/artifact_config.py (3 hunks)
- src/zenml/artifacts/external_artifact.py (2 hunks)
- src/zenml/artifacts/external_artifact_config.py (3 hunks)
- src/zenml/cli/init.py (1 hunks)
- src/zenml/cli/base.py (1 hunks)
- src/zenml/model/model.py (12 hunks)
- src/zenml/model/utils.py (5 hunks)
- src/zenml/new/pipelines/model_utils.py (2 hunks)
- src/zenml/new/pipelines/pipeline.py (12 hunks)
- src/zenml/new/pipelines/pipeline_decorator.py (4 hunks)
- src/zenml/new/steps/step_decorator.py (5 hunks)
- src/zenml/orchestrators/step_launcher.py (4 hunks)
- src/zenml/orchestrators/step_runner.py (4 hunks)
- tests/integration/functional/test_client.py (3 hunks)
Files skipped from review due to trivial changes (3)
- docs/book/getting-started/core-concepts.md
- examples/e2e_nlp/.copier-answers.yml
- src/zenml/cli/init.py
Files skipped from review as they are similar to previous changes (19)
- .github/workflows/update-templates-to-examples.yml
- docs/book/user-guide/advanced-guide/data-management/model-management.md
- docs/book/user-guide/advanced-guide/data-management/tagging.md
- docs/book/user-guide/advanced-guide/pipelining-features/fetch-metadata-within-steps.md
- docs/book/user-guide/starter-guide/structuring-a-project.md
- docs/book/user-guide/starter-guide/track-ml-models.md
- src/zenml/artifacts/artifact_config.py
- src/zenml/artifacts/external_artifact.py
- src/zenml/artifacts/external_artifact_config.py
- src/zenml/cli/base.py
- src/zenml/model/model.py
- src/zenml/model/utils.py
- src/zenml/new/pipelines/model_utils.py
- src/zenml/new/pipelines/pipeline.py
- src/zenml/new/pipelines/pipeline_decorator.py
- src/zenml/new/steps/step_decorator.py
- src/zenml/orchestrators/step_launcher.py
- src/zenml/orchestrators/step_runner.py
- tests/integration/functional/test_client.py
Additional comments: 22
examples/e2e_nlp/steps/deploying/save_model.py (1)
- 46-46: The update from
get_step_context().model_version
toget_step_context().model
aligns with the PR's objective to renameModelVersion
toModel
.examples/e2e_nlp/steps/promotion/promote_get_metrics.py (5)
- 58-58: The use of
get_step_context().model
instead ofget_step_context().model_version
is in line with the PR's renaming strategy.- 60-60: Retrieving current model metrics using the updated
model
attribute is correct and follows the PR's renaming convention.- 67-67: The adjustment in the retrieval of the latest model version metrics to use the
model
attribute is consistent with the PR's changes.- 76-76: Logging the latest model version metrics with the updated terminology is correct.
- 80-80: The updated log message correctly reflects the latest model version metrics.
examples/e2e_nlp/steps/promotion/promote_metric_compare_promoter.py (2)
- 83-83: The change to use
get_step_context().model
for promoting the model is consistent with the PR's renaming efforts.- 84-84: Setting the model stage using the updated
model
attribute aligns with the PR's changes.docs/book/user-guide/advanced-guide/data-management/logging-metadata.md (7)
- 9-9: The introduction correctly sets the context for logging metadata for artifacts and models, aligning with the PR's renaming efforts.
- 40-40: The section header has been updated to "Logging Metadata for Models," which is consistent with the PR's renaming strategy.
- 42-42: The explanation of model metadata now correctly refers to models instead of model versions, in line with the PR's changes.
- 44-44: The example provided for logging metadata for a model is updated to reflect the new terminology.
- 60-60: The comment within the code example correctly indicates that metadata is associated with the ZenML model, adhering to the PR's renaming convention.
- 70-70: The parameters
model_name
andmodel_version
in the code example are correctly used, although the PR's context suggests thatmodel_version
might be deprecated. This needs to be verified with the new Model class implementation.- 78-78: The conclusion of the example emphasizes the association of metadata with the model, which is in line with the PR's renaming efforts.
examples/e2e_nlp/run.py (7)
- 30-30: The import statement has been updated to reflect the new
Model
class, aligning with the PR's renaming strategy.- 201-201: The instantiation of the
Model
class with the new attributes is correct and follows the PR's renaming convention.- 209-209: The
pipeline_args
dictionary is updated to use "model" instead of "model_version," which is consistent with the PR's changes.- 222-222: The instantiation of the
Model
class withModelStages.LATEST
is correct and aligns with the PR's renaming efforts.- 223-223: The update of the
pipeline_args
dictionary to use the newmodel
key is consistent with the PR's renaming strategy.- 235-235: The instantiation of the
Model
class withModelStages("staging")
for the deploying pipeline condition is correct and follows the PR's renaming convention.- 239-239: The
pipeline_args
dictionary is correctly updated to use the newmodel
key for the deploying pipeline.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM now (one minor nit)
docs/book/user-guide/advanced-guide/data-management/model-management.md
Outdated
Show resolved
Hide resolved
…gement.md Co-authored-by: Alex Strick van Linschoten <strickvl@users.noreply.github.com>
@coderabbitai review |
…tps://github.com/zenml-io/zenml into feature/OSSK-342-rename-model-version-to-a-model
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 4
Configuration used: .coderabbit.yaml
Files selected for processing (37)
- .github/workflows/update-templates-to-examples.yml (4 hunks)
- docs/book/stacks-and-components/component-guide/model-registries/custom.md (4 hunks)
- docs/book/stacks-and-components/component-guide/model-registries/model-registries.md (1 hunks)
- docs/book/user-guide/advanced-guide/data-management/logging-metadata.md (5 hunks)
- docs/book/user-guide/advanced-guide/data-management/model-management.md (15 hunks)
- docs/book/user-guide/advanced-guide/pipelining-features/configure-steps-pipelines.md (4 hunks)
- docs/book/user-guide/production-guide/configure-pipeline.md (1 hunks)
- examples/e2e/steps/promotion/promote_with_metric_compare.py (3 hunks)
- src/zenml/init.py (2 hunks)
- src/zenml/artifacts/utils.py (1 hunks)
- src/zenml/cli/init.py (1 hunks)
- src/zenml/cli/base.py (1 hunks)
- src/zenml/cli/utils.py (3 hunks)
- src/zenml/config/step_configurations.py (2 hunks)
- src/zenml/integrations/mlflow/model_registries/mlflow_model_registry.py (9 hunks)
- src/zenml/metadata/lazy_load.py (3 hunks)
- src/zenml/model/lazy_load.py (2 hunks)
- src/zenml/model/model.py (15 hunks)
- src/zenml/model/utils.py (5 hunks)
- src/zenml/model_registries/base_model_registry.py (7 hunks)
- src/zenml/models/v2/core/artifact_version.py (3 hunks)
- src/zenml/models/v2/core/model.py (3 hunks)
- src/zenml/models/v2/core/model_version.py (3 hunks)
- src/zenml/models/v2/core/run_metadata.py (2 hunks)
- src/zenml/new/pipelines/pipeline.py (12 hunks)
- src/zenml/new/pipelines/pipeline_context.py (1 hunks)
- src/zenml/orchestrators/input_utils.py (2 hunks)
- src/zenml/orchestrators/step_launcher.py (5 hunks)
- src/zenml/orchestrators/step_runner.py (4 hunks)
- src/zenml/steps/base_step.py (12 hunks)
- src/zenml/zen_stores/migrations/versions/4d688d8f7aff_rename_model_version_to_model.py (1 hunks)
- tests/integration/functional/cli/test_model_registry.py (6 hunks)
- tests/integration/functional/model/test_model_version.py (30 hunks)
- tests/integration/functional/pipelines/test_pipeline_context.py (6 hunks)
- tests/integration/functional/test_client.py (3 hunks)
- tests/integration/functional/zen_stores/test_zen_store.py (43 hunks)
- tests/integration/functional/zen_stores/utils.py (1 hunks)
Files skipped from review as they are similar to previous changes (22)
- .github/workflows/update-templates-to-examples.yml
- docs/book/user-guide/advanced-guide/data-management/logging-metadata.md
- docs/book/user-guide/advanced-guide/data-management/model-management.md
- examples/e2e/steps/promotion/promote_with_metric_compare.py
- src/zenml/init.py
- src/zenml/cli/init.py
- src/zenml/cli/base.py
- src/zenml/config/step_configurations.py
- src/zenml/model/utils.py
- src/zenml/models/v2/core/model.py
- src/zenml/models/v2/core/model_version.py
- src/zenml/new/pipelines/pipeline.py
- src/zenml/new/pipelines/pipeline_context.py
- src/zenml/orchestrators/step_launcher.py
- src/zenml/orchestrators/step_runner.py
- src/zenml/steps/base_step.py
- src/zenml/zen_stores/migrations/versions/4d688d8f7aff_rename_model_version_to_model.py
- tests/integration/functional/model/test_model_version.py
- tests/integration/functional/pipelines/test_pipeline_context.py
- tests/integration/functional/test_client.py
- tests/integration/functional/zen_stores/test_zen_store.py
- tests/integration/functional/zen_stores/utils.py
Additional comments: 44
src/zenml/model/lazy_load.py (2)
- 20-20: The import statement has been updated to reflect the renaming of
ModelVersion
toModel
. This change is consistent with the PR objectives.- 31-31: The attribute
model
has been correctly updated frommodel_version
to align with the renaming across the codebase.src/zenml/metadata/lazy_load.py (4)
- 19-19: The import statement has been updated to reflect the renaming of
ModelVersion
toModel
. This change is consistent with the PR objectives.- 33-33: The parameter
_lazy_load_model
has been correctly updated from_lazy_load_model_version
to align with the renaming across the codebase.- 44-44: The attribute
_lazy_load_model
has been correctly updated from_lazy_load_model_version
to align with the renaming across the codebase.- 60-60: The parameter
_lazy_load_model
has been correctly updated from_lazy_load_model_version
to align with the renaming across the codebase.src/zenml/orchestrators/input_utils.py (4)
- 90-90: The method call
config_.model.get_artifact
has been correctly updated fromconfig_.model_version.get_artifact
to align with the renaming across the codebase.- 98-98: The attribute access
config_.model.run_metadata
has been correctly updated fromconfig_.model_version.run_metadata
to align with the renaming across the codebase.- 103-103: The method call
config_.model.get_artifact
has been correctly updated fromconfig_.model_version.get_artifact
to align with the renaming across the codebase.- 119-120: The error message has been correctly updated to use
config_.model.name
andconfig_.model.version
instead ofconfig_.model_version.name
andconfig_.model_version.version
.docs/book/user-guide/production-guide/configure-pipeline.md (1)
- 55-55: The YAML configuration field
model
has been correctly updated frommodel_version
to align with the renaming across the codebase.src/zenml/models/v2/core/run_metadata.py (2)
- 33-33: The import statement has been updated to reflect the renaming of
ModelVersion
toModel
. This change is consistent with the PR objectives.- 206-206: The attribute
_lazy_load_model
has been correctly updated from_lazy_load_model_version
to align with the renaming across the codebase.tests/integration/functional/cli/test_model_registry.py (1)
- 29-29: The import statement has been updated to reflect the renaming of
ModelVersion
toRegistryModelVersion
. This change is consistent with the PR objectives.src/zenml/models/v2/core/artifact_version.py (3)
- 47-47: The import statement has been updated to reflect the renaming of
ModelVersion
toModel
. This change is consistent with the PR objectives.- 480-480: The attribute
_lazy_load_model
has been correctly updated from_lazy_load_model_version
to align with the renaming across the codebase.- 510-510: The reference to
self._lazy_load_model
has been correctly updated fromself._lazy_load_model_version
within therun_metadata
method to align with the renaming across the codebase.src/zenml/model_registries/base_model_registry.py (6)
- 126-126: Renaming
ModelVersion
toRegistryModelVersion
is consistent with the PR objectives. Ensure that all references to the old class name are updated across the codebase.- 291-291: The return type of
register_model_version
has been correctly updated toRegistryModelVersion
. Ensure that all calls to this method are updated accordingly.- 336-336: The return type of
update_model_version
has been correctly updated toRegistryModelVersion
. Ensure that all calls to this method are updated accordingly.- 367-367: The return type of
list_model_versions
has been correctly updated toOptional[List[RegistryModelVersion]]
. Ensure that all calls to this method are updated accordingly.- 390-390: The return type of
get_latest_model_version
has been correctly updated toOptional[RegistryModelVersion]
. Ensure that all calls to this method are updated accordingly.- 414-415: The return type of
get_model_version
has been correctly updated toRegistryModelVersion
. Ensure that all calls to this method are updated accordingly.src/zenml/artifacts/utils.py (1)
- 236-238: The variable
model
has been correctly updated frommodel_version
to reflect the renaming. Ensure that the rest of the function logic is consistent with this change.src/zenml/integrations/mlflow/model_registries/mlflow_model_registry.py (8)
- 22-24: The import statement has been changed to import
RegistryModelVersion
asMLflowModelVersion
. This change aligns with the PR objectives of renamingModelVersion
toModel
across the codebase.- 43-43: The return type of several functions has been changed from
ModelVersion
toRegistryModelVersion
. This change is consistent with the renaming effort described in the PR objectives.- 353-353: The return type of the
register_model_version
method has been updated toRegistryModelVersion
. This is in line with the renaming effort and is correctly implemented.- 447-447: The return type of the
update_model_version
method has been updated toRegistryModelVersion
. This change is consistent with the renaming effort and is correctly implemented.- 526-526: The return type of the
get_model_version
method has been updated toRegistryModelVersion
. This change is consistent with the renaming effort and is correctly implemented.- 566-566: The return type of the
list_model_versions
method has been updated to a list ofRegistryModelVersion
. This change is consistent with the renaming effort and is correctly implemented.- 728-728: The
_cast_mlflow_version_to_model_version
method has been updated to returnRegistryModelVersion
. This change is consistent with the renaming effort and is correctly implemented.- 709-709: The
get_model_uri_artifact_store
method's parameter type has been updated toRegistryModelVersion
. This change is consistent with the renaming effort and is correctly implemented.src/zenml/model/model.py (9)
- 45-45: The class name has been changed from
ModelVersion
toModel
. This change is consistent with the PR objectives of renamingModelVersion
toModel
across the codebase.- 100-100: The method comments have been updated to reflect the change from
ModelVersion
toModel
. This change is consistent with the renaming effort and is correctly implemented.- 286-286: The
set_stage
method signature has been updated to acceptModel
instead ofModelVersion
. This change is consistent with the renaming effort and is correctly implemented.- 434-434: The
_lazy_artifact_get
method has been updated to create a newModel
instance with the current model's name and version or number. This change is consistent with the renaming effort and is correctly implemented.- 444-444: The
__eq__
method has been updated to compare instances ofModel
instead ofModelVersion
. This change is consistent with the renaming effort and is correctly implemented.- 512-512: The
_get_or_create_model
method has been updated to handle the renaming ofModelVersion
toModel
. This change is consistent with the renaming effort and is correctly implemented.- 598-598: The
_get_model_version
method has been updated to handle the renaming ofModelVersion
toModel
. This change is consistent with the renaming effort and is correctly implemented.- 666-666: The
_get_or_create_model_version
method has been updated to handle the renaming ofModelVersion
toModel
. This change is consistent with the renaming effort and is correctly implemented.- 724-724: The
_merge
method has been updated to handle the renaming ofModelVersion
toModel
. This change is consistent with the renaming effort and is correctly implemented.src/zenml/cli/utils.py (3)
- 62-62: Renaming
ModelVersion
toRegistryModelVersion
is consistent with the PR objectives.- 1185-1185: The parameter type hint for
model_versions
inpretty_print_model_version_table
has been updated toList["RegistryModelVersion"]
.- 1209-1209: The parameter type hint and name for
model_version
inpretty_print_model_version_details
have been updated toRegistryModelVersion
.
@@ -138,7 +138,7 @@ enable_artifact_visualization: Optional[bool] | |||
enable_cache: Optional[bool] | |||
enable_step_logs: Optional[bool] | |||
extra: Mapping[str, Any] | |||
model_version: | |||
model: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The term model
is used here without context which might confuse readers. Ensure that the documentation clearly explains the renaming from ModelVersion
to Model
and its implications.
@@ -198,7 +198,7 @@ | |||
attribute: Optional[str] | |||
module: str | |||
type: SourceType | |||
model_version: | |||
model: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The term model
is used here without context which might confuse readers. Ensure that the documentation clearly explains the renaming from ModelVersion
to Model
and its implications.
@@ -262,7 +262,7 @@ | |||
attribute: Optional[str] | |||
module: str | |||
type: SourceType | |||
model_version: | |||
model: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The term model
is used here without context which might confuse readers. Ensure that the documentation clearly explains the renaming from ModelVersion
to Model
and its implications.
@@ -338,7 +338,7 @@ | |||
|
|||
This is a dictionary that is available to be passed to steps and pipelines called `extra`. This dictionary is meant to be used to pass any configuration down to the pipeline, step, or stack components that the user has use of. See an example in [this section](#fetching-configuration). | |||
|
|||
### Configuring the `model_version` | |||
### Configuring the `model` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The term model
is used here without context which might confuse readers. Ensure that the documentation clearly explains the renaming from ModelVersion
to Model
and its implications.
E2E template updates in |
…tps://github.com/zenml-io/zenml into feature/OSSK-342-rename-model-version-to-a-model
* big rename * more renaming leftovers * few more renames * add migration * resolve branching * bump templates * Auto-update of Starter template * Auto-update of E2E template * coderabbitai nits * fix template ci * update CI as in zenml-io#2221 * Auto-update of NLP template * some leftover renaming * remove annoying logger * remove branching * clean-up after merge * fix logging * rename `RegistryModelVersion` * lint * Update docs/book/user-guide/advanced-guide/data-management/model-management.md Co-authored-by: Alex Strick van Linschoten <strickvl@users.noreply.github.com> * fix renaming * Auto-update of E2E template * update test signatures --------- Co-authored-by: GitHub Actions <actions@github.com> Co-authored-by: Alex Strick van Linschoten <strickvl@users.noreply.github.com>
* big rename * more renaming leftovers * few more renames * add migration * resolve branching * bump templates * Auto-update of Starter template * Auto-update of E2E template * coderabbitai nits * fix template ci * update CI as in zenml-io#2221 * Auto-update of NLP template * some leftover renaming * remove annoying logger * remove branching * clean-up after merge * fix logging * rename `RegistryModelVersion` * lint * Update docs/book/user-guide/advanced-guide/data-management/model-management.md Co-authored-by: Alex Strick van Linschoten <strickvl@users.noreply.github.com> * fix renaming * Auto-update of E2E template * update test signatures --------- Co-authored-by: GitHub Actions <actions@github.com> Co-authored-by: Alex Strick van Linschoten <strickvl@users.noreply.github.com>
Describe changes
This is a big renaming PR moving from ModelVersion to Model naming across the board.
Breaking change:
@pipeline(model_version=ModelVersion(...))
->@pipeline(model=Model(...))
@step(model_version=ModelVersion(...))
->@step(model=Model(...))
zenml.model_registries.base_model_registry.ModelVersion
->zenml.model_registries.base_model_registry.RegistryModelVersion
zenml.integration,mlflow.model_registries.base_model_registry.ModelVersion
->zenml.integration,mlflow.model_registries.base_model_registry.RegistryModelVersion
Pre-requisites
Please ensure you have done the following:
develop
and the open PR is targetingdevelop
. If your branch wasn't based on develop read Contribution guide on rebasing branch to develop.Types of changes
Summary by CodeRabbit
Summary by CodeRabbit
Documentation
ModelVersion
toModel
.New Features
Model
naming convention.Refactor
ModelVersion
toModel
across the codebase to simplify model handling.Tests
Model
terminology.Chores
model_version
tomodel
.