Skip to content
New issue

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

Sign up for GitHub

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

Restore call convention compatibility in get_model #304

Merged
merged 1 commit into from
Jul 3, 2020
Merged

Restore call convention compatibility in get_model #304

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 30 additions & 15 deletions diabetes_regression/util/model_helper.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@

def get_current_workspace() -> Workspace:
"""
Retrieves and returns the latest model from the workspace
by its name and tag. Will not work when ran locally.
Retrieves and returns the current workspace.
Will not work when ran locally.

Parameters:
None
Expand All @@ -30,8 +30,8 @@ def get_model(
aml_workspace: Workspace = None
) -> AMLModel:
"""
Retrieves and returns the latest model from the workspace
by its name and (optional) tag.
Retrieves and returns a model from the workspace by its name
and (optional) tag.

Parameters:
aml_workspace (Workspace): aml.core Workspace that the model lives.
Expand All @@ -40,25 +40,40 @@ def get_model(
(optional) tag (str): the tag value & name the model was registered under.

Return:
A single aml model from the workspace that matches the name and tag.
A single aml model from the workspace that matches the name and tag, or
None.
"""
if aml_workspace is None:
print("No workspace defined - using current experiment workspace.")
aml_workspace = get_current_workspace()

if tag_name is not None and tag_value is not None:
tags = None
if tag_name is not None or tag_value is not None:
# Both a name and value must be specified to use tags.
if tag_name is None or tag_value is None:
raise ValueError(
"model_tag_name and model_tag_value should both be supplied"
+ "or excluded" # NOQA: E501
)
tags = [[tag_name, tag_value]]

model = None
if model_version is not None:
# TODO(tcare): Finding a specific version currently expects exceptions
# to propagate in the case we can't find the model. This call may
# result in a WebserviceException that may or may not be due to the
# model not existing.
Comment on lines +62 to +65
Copy link
Contributor

Choose a reason for hiding this comment

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

Deployment isn't using this method - why WebserviceException? It should be just the training and batch scoring pipeline that uses this.

Copy link
Contributor

Choose a reason for hiding this comment

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

Nevermind - I see what you're saying. Can you make a new issue for this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will do :)

model = AMLModel(
aml_workspace,
name=model_name,
version=model_version,
tags=[[tag_name, tag_value]])
elif (tag_name is None and tag_value is not None) or (
tag_value is None and tag_name is not None
):
raise ValueError(
"model_tag_name and model_tag_value should both be supplied"
+ "or excluded" # NOQA: E501
)
tags=tags)
else:
model = AMLModel(aml_workspace, name=model_name, version=model_version) # NOQA: E501
models = AMLModel.list(
aml_workspace, name=model_name, tags=tags, latest=True)
if len(models) == 1:
model = models[0]
elif len(models) > 1:
raise Exception("Expected only one model")

return model