Skip to content

Merge master into branch-2.0#6686

Merged
dbczumar merged 6 commits intomlflow:branch-2.0from
dbczumar:merge_master_20
Sep 6, 2022
Merged

Merge master into branch-2.0#6686
dbczumar merged 6 commits intomlflow:branch-2.0from
dbczumar:merge_master_20

Conversation

@dbczumar
Copy link
Copy Markdown
Collaborator

@dbczumar dbczumar commented Sep 2, 2022

Signed-off-by: dbczumar [email protected]

What changes are proposed in this pull request?

Merge master into branch-2.0

How is this patch tested?

Existing CI

Does this PR change the documentation?

  • No. You can skip the rest of this section.
  • Yes. Make sure the changed pages / sections render correctly by following the steps below.
  1. Click the Details link on the Preview docs check.
  2. Find the changed pages / sections and make sure they render correctly.

Release Notes

Is this a user-facing change?

  • No. You can skip the rest of this section.
  • Yes. Give a description of this change to be included in the release notes for MLflow users.

(Details in 1-2 sentences. You can just refer to another PR with a description if this PR is part of a larger change.)

What component(s), interfaces, languages, and integrations does this PR affect?

Components

  • area/artifacts: Artifact stores and artifact logging
  • area/build: Build and test infrastructure for MLflow
  • area/docs: MLflow documentation pages
  • area/examples: Example code
  • area/model-registry: Model Registry service, APIs, and the fluent client calls for Model Registry
  • area/models: MLmodel format, model serialization/deserialization, flavors
  • area/pipelines: Pipelines, Pipeline APIs, Pipeline configs, Pipeline Templates
  • area/projects: MLproject format, project running backends
  • area/scoring: MLflow Model server, model deployment tools, Spark UDFs
  • area/server-infra: MLflow Tracking server backend
  • area/tracking: Tracking Service, tracking client APIs, autologging

Interface

  • area/uiux: Front-end, user experience, plotting, JavaScript, JavaScript dev server
  • area/docker: Docker use across MLflow's components, such as MLflow Projects and MLflow Models
  • area/sqlalchemy: Use of SQLAlchemy in the Tracking Service or Model Registry
  • area/windows: Windows support

Language

  • language/r: R APIs and clients
  • language/java: Java APIs and clients
  • language/new: Proposals for new client languages

Integrations

  • integrations/azure: Azure and Azure ML integrations
  • integrations/sagemaker: SageMaker integrations
  • integrations/databricks: Databricks integrations

How should the PR be classified in the release notes? Choose one:

  • rn/breaking-change - The PR will be mentioned in the "Breaking Changes" section
  • rn/none - No description will be included. The PR will be mentioned only by the PR number in the "Small Bugfixes and Documentation Updates" section
  • rn/feature - A new user-facing feature worth mentioning in the release notes
  • rn/bug-fix - A user-facing bug fix worth mentioning in the release notes
  • rn/documentation - A user-facing documentation change worth mentioning in the release notes

@dbczumar
Merge master
6b8f062 
Signed-off-by: dbczumar <[email protected]>
@dbczumar
Remove dupe arg
9022381 
Signed-off-by: dbczumar <[email protected]>
harupy
harupy reviewed Sep 2, 2022
View reviewed changes
mlflow/sagemaker/__init__.py Outdated
}
mfs.deploy(..., vpc_config=vpc_config)

:param data_capture_config: A dictionary specifying the data capture configuration to use when
Copy link
Copy Markdown
Member

@harupy harupy Sep 2, 2022

Choose a reason for hiding this comment

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

Can we remove this? We have two data_capture_config param descriptions.

Copy link
Copy Markdown
Collaborator Author

@dbczumar dbczumar Sep 2, 2022

Choose a reason for hiding this comment

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

Removed! :)

@harupy
Copy link
Copy Markdown
Member

harupy commented Sep 2, 2022

Should we revert the changes in examples/pipelines/sklearn_regression?

harupy
harupy reviewed Sep 2, 2022
View reviewed changes
tests/models/test_evaluation.py
Comment on lines -647 to -649
class PyFuncModelMatcher:
def __eq__(self, other):
return isinstance(other, mlflow.pyfunc.PyFuncModel)
Copy link
Copy Markdown
Member

@harupy harupy Sep 2, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Let's revert this :)

Copy link
Copy Markdown
Member

@harupy harupy Sep 2, 2022

Choose a reason for hiding this comment

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

Can we revert this? This class is used in test_evaluator_evaluation_interface.

Copy link
Copy Markdown
Collaborator Author

@dbczumar dbczumar Sep 3, 2022

Choose a reason for hiding this comment

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

Thanks for catching this - reverted!

@github-actions github-actions bot added area/build Build and test infrastructure for MLflow rn/none List under Small Changes in Changelogs. labels Sep 2, 2022
harupy
harupy reviewed Sep 2, 2022
View reviewed changes
tests/models/test_evaluation.py Outdated
classifier_model = mlflow.pyfunc.load_model(multiclass_logistic_regressor_model_uri)
with expected_error:
evaluate(
classifier_model,
Copy link
Copy Markdown
Member

@harupy harupy Sep 2, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Suggested change
classifier_model,
multiclass_logistic_regressor_model_uri,

I think this line needs to be fixed. evaluate no longer accepts a pyfunc model.

Copy link
Copy Markdown
Collaborator Author

@dbczumar dbczumar Sep 3, 2022

Choose a reason for hiding this comment

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

Fixed!

harupy
harupy approved these changes Sep 2, 2022
View reviewed changes
Copy link
Copy Markdown
Member

@harupy harupy left a comment

Choose a reason for hiding this comment

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

LGTM once remaining comments are addressed!

@harupy
Copy link
Copy Markdown
Member

harupy commented Sep 2, 2022 •
edited
Loading

We can remove files in tests/pylint_plugins (which caused some CI checks to fail).

@dbczumar
Copy link
Copy Markdown
Collaborator Author

dbczumar commented Sep 3, 2022

Should we revert the changes in examples/pipelines/sklearn_regression?

No, we should update the pipelines submodule to the same commit used on master, which this PR does.

@dbczumar
Copy link
Copy Markdown
Collaborator Author

dbczumar commented Sep 3, 2022

tests/pylint_plugins

Thanks! Removed!

@dbczumar
Address comments, fix tests
4849bb1 
Signed-off-by: dbczumar <[email protected]>
@dbczumar
Fix tests
39c0dcc 
Signed-off-by: dbczumar <[email protected]>
@dbczumar
Fix
eec95df 
Signed-off-by: dbczumar <[email protected]>
@dbczumar dbczumar merged commit 72f12b5 into mlflow:branch-2.0 Sep 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@harupy harupy harupy approved these changes

@WeichenXu123 WeichenXu123 Awaiting requested review from WeichenXu123

@BenWilson2 BenWilson2 Awaiting requested review from BenWilson2

Assignees

No one assigned

Labels

area/build Build and test infrastructure for MLflow rn/none List under Small Changes in Changelogs.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@dbczumar @harupy