Do not modify tags dict on start_run()

[matheusMoreno]

What changes are proposed in this pull request?

On the start_run() function, make a deep copy of the tags dictionary and do not redefine the dictionary. Fixes #5190.

How is this patch tested?

Installed MLflow from my branch, and ran the sample code in #5190. It did not raise any errors.

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.

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.

Fixes bug where dictionary variables for tags were being altered, causing runs to be incorrectly considered nested.

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/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

[github-actions]

@matheusMoreno Thanks for the contribution! The DCO check failed. Please sign off your commits by following the instructions here: https://github.com/mlflow/mlflow/runs/4599678833. See https://github.com/mlflow/mlflow/blob/master/CONTRIBUTING.rst#sign-your-work for more details.

[harupy]

@matheusMoreno Thanks for the PR! Can we also fix this line?

mlflow/mlflow/tracking/_tracking_service/client.py

Line 97 in b6eaafd

tags = tags if tags else {}

[matheusMoreno]

@matheusMoreno Thanks for the PR! Can we also fix this line?

mlflow/mlflow/tracking/_tracking_service/client.py

Line 97 in b6eaafd

tags = tags if tags else {}

Fixed on 355fcd4. Did not use deep copy because the function only reads the dict, with no modifications.

Also renamed the variable on start_run() as suggested, and did not remove the deepcopy() call.

[harupy]

Did not use deep copy because the function only reads the dict, with no modifications.

Ah, you're right. I didn't look at the code carefully.

[harupy]

Do we need this change?

[matheusMoreno]

At first I thought so, since it could rewrite the parameter, but it does seem unnecessary. Will undo the commit!

[harupy]

LGTM!