Supporting kwargs in log figure

[stroblme]

Related Issues/PRs

Resolve #9149

What changes are proposed in this pull request?

• added **kwargs to log_figure(..) and passed them to corresponding export methods for both matplotlib and plotly
• using setdefault to merge previously existing arguments in kwargs
• updated docstring

How is this patch tested?

• Existing unit/integration tests

• New unit/integration tests

• Manual tests (describe details, including test results, below)

• ran only tracking related unit tests (pytest tests/tracking/test_log_figure.py)

• manual test code based on example provided in method documentation:

from mlflow import MlflowClient
from plotly import graph_objects as go

client = MlflowClient()

fig = go.Figure(go.Scatter(x=[0, 1], y=[2, 3]))
fig.update_layout(
    title={
        'text': r"$\text{A line with slope}\quad \frac{3-2}{1-0}=1$",
    })

run = client.create_run(experiment_id="0")
client.log_figure(run.info.run_id, fig, "figure.html", include_mathjax="cdn")

• manual test results (with and without include_mathjax="cdn") :
  
  

Does this PR change the documentation?

• No. You can skip the rest of this section.
• Yes. Make sure the changed pages / sections render correctly in the documentation preview.

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.)

Additional arguments can be passed to both Matplotlib and Plotly export methods. This enables specifying e.g. MathJax CDN
required for Plotly figures with formulas as mentioned in #9149

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/gateway: AI Gateway service, Gateway client APIs, third-party Gateway integrations
• area/model-registry: Model Registry service, APIs, and the fluent client calls for Model Registry
• area/models: MLmodel format, model serialization/deserialization, flavors
• area/recipes: Recipes, Recipe APIs, Recipe configs, Recipe 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

[mlflow-automation]

Documentation preview for d8b9e02 will be available here when this CircleCI job completes successfully.

More info

• Ignore this comment if this PR does not change the documentation.
• It takes a few minutes for the preview to be available.
• The preview is updated when a new commit is pushed to this PR.
• This comment was created by https://github.com/mlflow/mlflow/actions/runs/5853264781.

[harupy]

Suggested change

	**kwargs,
	*,
	kwargs,

Can we make this a separate argument? In the future, we might add a new argument to this function and it may conflict with plotly or matplotlib. I think this is a little inconvenient but better than argument name collisions.

[stroblme]

I see, good point! Should we then also rename kwargs to sth. more expressive, e.g. figure_args or plotting_args (open to other suggestions), so that it is clear that those arguments are being passed to the plotting backend?

[harupy]

xxx_args sounds like it takes a tuple. I'd prefer xxx_kwargs.

• figure_kwargs
• plot_kwargs
• plotting_kwargs
• extra_kwargs
• save_kwargs
• extras

[harupy]

save_kwargs (meaning keyword arguments used when saving)?

[harupy]

kwargs doesn't sound bad because we probably won't add an argument named kwargs to log_figure.

[stroblme]

save_kwargs sound good, the other suggestions like figure_kwargs might cause confusion as it could be interpreted as altering the figure itself. Also sticking with kwargs is fine by me 😃
Is there maybe another (similar) case in the mlflow api where we could reuse the naming convention for additional kwargs passed to the backend?

[harupy]

I like save_kwargs :) It's more descriptive than kwargs. We have saved_model_kwargs here in the mlflow.tensorflow module:

mlflow/mlflow/tensorflow/__init__.py

Line 199 in 0e856e2

:param saved_model_kwargs: a dict of kwargs to pass to ``tensorflow.saved_model.save`` method.

save_kwargs aligns with this pattern.

[harupy]

Suggested change

[harupy]

@stroblme Sorry for the late review, left a few comments!

[harupy]

@stroblme I pushed a commit to use save_kwargs. Let me know if it looks ok :)

[stroblme]

Thanks, looks good!
Good idea to add a test as well, wasn't sure how to check if it actually renders the Latex, but at least the interface is tested now.

[harupy]

LGTM!

[harupy]

@stroblme Thanks for the contribution!