plotly / plotly.py

The interactive graphing library for Python :sparkles: This project now includes Plotly Express!
https://plotly.com/python/
MIT License
15.98k stars 2.53k forks source link

[Typing] `px.timeline.color_continuous_scale` accepts also `list[tuple[float,str]]`, not only `list[str]` #4069

Open stdedos opened 1 year ago

stdedos commented 1 year ago

https://plotly.com/python-api-reference/generated/plotly.express.timeline.html

color_continuous_scale (list of str) – Strings should define valid CSS-colors This list is used to build a continuous color scale when the column denoted by color contains numeric data. Various useful color scales are available in the plotly.express.colors submodules, specifically plotly.express.colors.sequential, plotly.express.colors.diverging and plotly.express.colors.cyclical.

However, here the example shows:

https://github.com/plotly/plotly.py/blob/182fff29bce98c30ab516242d4e01d2d2d0bee0f/doc/python/colorscales.md#explicitly-constructing-a-color-scale

itsmegarvi commented 1 year ago

So, we need to change color_continuous_scale (list of str) to color_continuous_scale (list of str and list of a tuple of float and string). If I have missed something please guide me.

stdedos commented 1 year ago

Here's the syntactic type I've defined:

PlotlyColorScale = Union[list[str], list[tuple[Union[float, int], str]]]
# or, for py3.10/3.11
PlotlyColorScale = list[str] | list[tuple[float, str]]]
# ... but for mypy, it should be `float == float | int` (https://github.com/python/mypy/issues/3186)
nicolaskruchten commented 1 year ago

Yes, the docstring here is a bit behind the API expansion, but the error message you get if you give it something wrong is complete and correct, as below. Note that a simple string is accepted as well (it is looked up case-insensitively in a list of constants shown below), with or without a trailing "_r" to reverse it.

The 'colorscale' property is a colorscale and may be
    specified as:
      - A list of colors that will be spaced evenly to create the colorscale.
        Many predefined colorscale lists are included in the sequential, diverging,
        and cyclical modules in the plotly.colors package.
      - A list of 2-element lists where the first element is the
        normalized color level value (starting at 0 and ending at 1),
        and the second item is a valid color string.
        (e.g. [[0, 'green'], [0.5, 'red'], [1.0, 'rgb(0, 0, 255)']])
      - One of the following named colorscales:
            ['aggrnyl', 'agsunset', 'algae', 'amp', 'armyrose', 'balance',
             'blackbody', 'bluered', 'blues', 'blugrn', 'bluyl', 'brbg',
             'brwnyl', 'bugn', 'bupu', 'burg', 'burgyl', 'cividis', 'curl',
             'darkmint', 'deep', 'delta', 'dense', 'earth', 'edge', 'electric',
             'emrld', 'fall', 'geyser', 'gnbu', 'gray', 'greens', 'greys',
             'haline', 'hot', 'hsv', 'ice', 'icefire', 'inferno', 'jet',
             'magenta', 'magma', 'matter', 'mint', 'mrybm', 'mygbm', 'oranges',
             'orrd', 'oryel', 'oxy', 'peach', 'phase', 'picnic', 'pinkyl',
             'piyg', 'plasma', 'plotly3', 'portland', 'prgn', 'pubu', 'pubugn',
             'puor', 'purd', 'purp', 'purples', 'purpor', 'rainbow', 'rdbu',
             'rdgy', 'rdpu', 'rdylbu', 'rdylgn', 'redor', 'reds', 'solar',
             'spectral', 'speed', 'sunset', 'sunsetdark', 'teal', 'tealgrn',
             'tealrose', 'tempo', 'temps', 'thermal', 'tropic', 'turbid',
             'turbo', 'twilight', 'viridis', 'ylgn', 'ylgnbu', 'ylorbr',
             'ylorrd'].
        Appending '_r' to a named colorscale reverses it.
stdedos commented 1 year ago

Yes, the docstring here is a bit behind the API expansion, but the error message you get if you give it something wrong is complete and correct,

Well ... REPL is not the nicest way to develop with unknown libraries 😅

  1. typing/stubbing would've been amazing (https://github.com/plotly/plotly.py/issues/1103, https://github.com/plotly/plotly.py/issues/3401, (https://github.com/plotly/dash/issues/2233), ...)
  2. then it's documentation
  3. ... then it's pulling your hair "why does documentation say XYZ, but my developer here did ABC ⁉️ ⁉️ ⁉️ ... and it's working???"
  4. Then ... it's giving wrong values as input so that you can get an error message with the accepted types? While I commend your effort, on code that's working, that does not provide the same value (as when developing)
nicolaskruchten commented 1 year ago

Yep, there is definitely room for improvement here. My feeling is that maintaining type-hint information is much harder than keeping the docstrings up to date, and we're already struggling to keep the docstrings up to date, so I can't commit to adding and maintaining type hints at the moment. We would gladly accept a PR that makes the docstrings from Plotly Express more complete. The relevant file is here: https://github.com/plotly/plotly.py/blob/master/packages/python/plotly/plotly/express/_doc.py#L316

stdedos commented 1 year ago

My feeling is that maintaining type-hint information is much harder than keeping the docstrings up to date, ...

We certainly disagree on that 😅

pre-commit + mypy (especially now that it moved to v1.0.0) can go a long way of keeping the code up-to-date. Documentation is text; there's no validating it, certifying it, "tying it down".

What led me here is "flexible usage" of the plotly API, lack of documentation (and also flexible usage of my code base before calling the said API).

mypy is certainly a difficult demon to master, especially when supporting all "smart" cases - but it may be so just because it is actually solving the complex problem of "over-flexibility allowed" 🥵