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.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add type annotations for plotly_chart #4684

Merged
merged 7 commits into from
May 6, 2022
Merged
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
76 changes: 60 additions & 16 deletions lib/streamlit/elements/plotly_chart.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,18 +15,38 @@
"""Streamlit support for Plotly charts."""

import json
import sys
import urllib.parse
from typing import cast
from typing import Any, cast, Dict, List, Set, TYPE_CHECKING, Union

if sys.version_info >= (3, 8):
from typing import Final, Literal
else:
from typing_extensions import Final, Literal

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@vdonato I changed the imports here to be python version dependent. In the process I discovered that my used of TypeAlias actually does require typing_extensions to be installed almost unconditionally.

I thought I'd mention it as input to your decision process regarding that dependency.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks yet again @harahu!

If we could bug you to make one more batch of changes to these PRs, I think that now that typing_extensions is always installed, we might as well remove the branching imports here (I'll make a quick PR removing them in the ~2 places where they already exist, so no need to worry about those).

You mentioned in a comment in another PR that having these branching imports even with typing_extensions always installed may be reasonable as it gives us a signal as to when the imports can be made from the main typing package, but I feel like this isn't worth the added complexity. I also feel like it'll most likely never be problematic if we entirely forget to change these imports to be from typings > typing_extensions as we drop support for older versions of Python (and if types get removed entirely from typing_extensions, we'll catch it in CI).

I'll go ahead and approve the currently open PRs and can merge them as these last changes are made.

if sys.version_info >= (3, 10):
from typing import TypeAlias
else:
from typing_extensions import TypeAlias

import streamlit
from streamlit.legacy_caching import caching
from streamlit import type_util
from streamlit.logger import get_logger
from streamlit.proto.PlotlyChart_pb2 import PlotlyChart as PlotlyChartProto

LOGGER = get_logger(__name__)
if TYPE_CHECKING:
import matplotlib
import plotly.graph_objs as go
from plotly.basedatatypes import BaseFigure

from streamlit.delta_generator import DeltaGenerator


SHARING_MODES = {
LOGGER: Final = get_logger(__name__)

SharingMode: TypeAlias = Literal["streamlit", "private", "public", "secret"]

SHARING_MODES: Set[SharingMode] = {
# This means the plot will be sent to the Streamlit app rather than to
# Plotly.
"streamlit",
Expand All @@ -37,15 +57,30 @@
"secret",
}

_AtomicFigureOrData: TypeAlias = Union[
"go.Figure",
"go.Data",
]
FigureOrData: TypeAlias = Union[
_AtomicFigureOrData,
List[_AtomicFigureOrData],
# It is kind of hard to figure out exactly what kind of dict is supported
# here, as plotly hasn't embraced typing yet. This version is chosen to
# align with the docstring.
Dict[str, _AtomicFigureOrData],
"BaseFigure",
"matplotlib.figure.Figure",
]


class PlotlyMixin:
def plotly_chart(
self,
figure_or_data,
use_container_width=False,
sharing="streamlit",
**kwargs,
):
figure_or_data: FigureOrData,
use_container_width: bool = False,
sharing: SharingMode = "streamlit",
**kwargs: Any,
) -> "DeltaGenerator":
"""Display an interactive Plotly chart.

Plotly is a charting library for Python. The arguments to this function
Expand Down Expand Up @@ -115,15 +150,24 @@ def plotly_chart(
marshall(
plotly_chart_proto, figure_or_data, use_container_width, sharing, **kwargs
)
return self.dg._enqueue("plotly_chart", plotly_chart_proto)
return cast(
"DeltaGenerator",
self.dg._enqueue("plotly_chart", plotly_chart_proto),
)

@property
def dg(self) -> "streamlit.delta_generator.DeltaGenerator":
def dg(self) -> "DeltaGenerator":
"""Get our DeltaGenerator."""
return cast("streamlit.delta_generator.DeltaGenerator", self)
return cast("DeltaGenerator", self)


def marshall(proto, figure_or_data, use_container_width, sharing, **kwargs):
def marshall(
proto: PlotlyChartProto,
figure_or_data: FigureOrData,
use_container_width: bool,
sharing: SharingMode,
**kwargs: Any,
) -> None:
"""Marshall a proto with a Plotly spec.

See DeltaGenerator.plotly_chart for docs.
Expand Down Expand Up @@ -166,10 +210,10 @@ def marshall(proto, figure_or_data, use_container_width, sharing, **kwargs):


@caching.cache
def _plot_to_url_or_load_cached_url(*args, **kwargs):
def _plot_to_url_or_load_cached_url(*args: Any, **kwargs: Any) -> "go.Figure":
"""Call plotly.plot wrapped in st.cache.

This is so we don't unecessarily upload data to Plotly's SASS if nothing
This is so we don't unnecessarily upload data to Plotly's SASS if nothing
changed since the previous upload.
"""
try:
Expand All @@ -181,7 +225,7 @@ def _plot_to_url_or_load_cached_url(*args, **kwargs):
return ply.plot(*args, **kwargs)


def _get_embed_url(url):
def _get_embed_url(url: str) -> str:
parsed_url = urllib.parse.urlparse(url)

# Plotly's embed URL is the normal URL plus ".embed".
Expand Down