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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug] Fix display of marks in vm.Slider and vm.RangeSlider #613

Merged
merged 15 commits into from
Aug 2, 2024
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
<!--
A new scriv changelog fragment.

Uncomment the section that is right (remove the HTML comment wrapper).
-->

<!--
### Highlights ✨

- A bullet item for the Highlights ✨ category with a link to the relevant PR at the end of your entry, e.g. Enable feature XXX ([#1](https://github.com/mckinsey/vizro/pull/1))

-->
<!--
### Removed

- A bullet item for the Removed category with a link to the relevant PR at the end of your entry, e.g. Enable feature XXX ([#1](https://github.com/mckinsey/vizro/pull/1))

-->
<!--
### Added

- A bullet item for the Added category with a link to the relevant PR at the end of your entry, e.g. Enable feature XXX ([#1](https://github.com/mckinsey/vizro/pull/1))

-->
<!--
### Changed

- A bullet item for the Changed category with a link to the relevant PR at the end of your entry, e.g. Enable feature XXX ([#1](https://github.com/mckinsey/vizro/pull/1))

-->
<!--
### Deprecated

- A bullet item for the Deprecated category with a link to the relevant PR at the end of your entry, e.g. Enable feature XXX ([#1](https://github.com/mckinsey/vizro/pull/1))

-->

### Fixed

- Fix display of marks in `vm.Slider` and `vm.RangeSlider` by converting floats to integers when possible. ([#613](https://github.com/mckinsey/vizro/pull/613))

<!--
### Security

- A bullet item for the Security category with a link to the relevant PR at the end of your entry, e.g. Enable feature XXX ([#1](https://github.com/mckinsey/vizro/pull/1))

-->
95 changes: 45 additions & 50 deletions vizro-core/examples/scratch_dev/app.py
Original file line number Diff line number Diff line change
@@ -1,64 +1,59 @@
"""Dev app to try things out."""

from typing import List
from typing import Any, Literal

import pandas as pd
import plotly.graph_objects as go
import vizro.models as vm
from dash import dcc
from vizro import Vizro
from vizro.models.types import capture

sankey_data = pd.DataFrame(
{
"Origin": [0, 1, 2, 1, 2, 4, 0], # indices inside labels
"Destination": [1, 2, 3, 4, 5, 5, 6], # indices inside labels
"Value": [10, 4, 8, 6, 4, 8, 8],
}
)


@capture("graph")
def sankey(
data_frame: pd.DataFrame,
source: str,
target: str,
value: str,
labels: List[str],
) -> go.Figure:
"""Creates a sankey diagram based on a go.Figure."""
fig = go.Figure(
data=[
go.Sankey(
node={
"pad": 16,
"thickness": 16,
"label": labels,
},
link={
"source": data_frame[source],
"target": data_frame[target],
"value": data_frame[value],
"label": labels,
"color": "rgba(205, 209, 228, 0.4)",
},
)
]
)
fig.update_layout(barmode="relative")
return fig
class PureDashSlider(vm.VizroBaseModel):
"""Simple Dash Slider."""

type: Literal["simple_slider"] = "simple_slider"
kwargs: Any

def build(self):
"""Pure Slider component."""
return dcc.Slider(**self.kwargs)


vm.Container.add_type("components", vm.Slider)
vm.Container.add_type("components", PureDashSlider)

# All floats: This works on Vizro, while it does not work in Dash
a = dict(min=0, max=2, step=0.1, marks={0.0: "a", 1.0: "x", 2.0: "y"}) # noqa: C408

# All int: This works in both Vizro and Dash
b = dict(min=0, max=2, step=0.1, marks={0: "a", 1: "x", 2: "y"}) # noqa: C408

# Mixed float and int: This works in Vizro, while it only partially works in Dash
c = dict(min=0, max=1, step=0.1, marks={0: "a", 0.5: "x", 1.0: "y"}) # noqa: C408

# User example
e = dict(min=0, max=1, step=0.01, marks={0: "0%", 0.21: "MARKET", 1.0: "100%"}) # noqa: C408

# Other test example: https://github.com/mckinsey/vizro/pull/266
d = dict(min=2, max=5, step=1, value=3) # noqa: C408

page = vm.Page(
title="Sankey",
title="Vizro on PyCafe",
components=[
vm.Graph(
figure=sankey(
data_frame=sankey_data,
labels=["A1", "A2", "B1", "B2", "C1", "C2", "D1"],
source="Origin",
target="Destination",
value="Value",
),
vm.Container(
title="vm.Sliders",
layout=vm.Layout(grid=[[0, 1, 2, 3, 4]]),
components=[vm.Slider(**a), vm.Slider(**b), vm.Slider(**c), vm.Slider(**d), vm.Slider(**e)],
),
vm.Container(
title="dcc.Sliders",
layout=vm.Layout(grid=[[0, 1, 2, 3, 4]]),
components=[
PureDashSlider(kwargs=a),
PureDashSlider(kwargs=b),
PureDashSlider(kwargs=c),
PureDashSlider(kwargs=d),
PureDashSlider(kwargs=e),
],
),
],
)
Expand Down
18 changes: 2 additions & 16 deletions vizro-core/schemas/0.1.20.dev0.json
Original file line number Diff line number Diff line change
Expand Up @@ -857,14 +857,7 @@
"default": {},
"type": "object",
"additionalProperties": {
"anyOf": [
{
"type": "string"
},
{
"type": "object"
}
]
"type": "string"
}
},
"value": {
Expand Down Expand Up @@ -932,14 +925,7 @@
"default": {},
"type": "object",
"additionalProperties": {
"anyOf": [
{
"type": "string"
},
{
"type": "object"
}
]
"type": "string"
}
},
"value": {
Expand Down
6 changes: 6 additions & 0 deletions vizro-core/src/vizro/models/_components/form/_form_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,12 @@ def validate_step(cls, step, values):
def set_default_marks(cls, marks, values):
if not marks and values.get("step") is None:
marks = None

# Dash has a bug where marks provided as floats that can be converted to integers are not displayed.
# So we need to convert the floats to integers if possible.
# https://github.com/plotly/dash-core-components/issues/159#issuecomment-380581043
if marks:
marks = {int(k) if k.is_integer() else k: v for k, v in marks.items()}
return marks


Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
from typing import Any, Dict, List, Literal, Optional, Union
from typing import Dict, List, Literal, Optional

from dash import ClientsideFunction, Input, Output, State, clientside_callback, dcc, html

Expand Down Expand Up @@ -43,7 +43,7 @@ class RangeSlider(VizroBaseModel):
min: Optional[float] = Field(None, description="Start value for slider.")
max: Optional[float] = Field(None, description="End value for slider.")
step: Optional[float] = Field(None, description="Step-size for marks on slider.")
marks: Optional[Dict[int, Union[str, Dict[str, Any]]]] = Field({}, description="Marks to be displayed on slider.")
marks: Optional[Dict[float, str]] = Field({}, description="Marks to be displayed on slider.")
value: Optional[List[float]] = Field(
None, description="Default start and end value for slider", min_items=2, max_items=2
)
Expand Down
4 changes: 2 additions & 2 deletions vizro-core/src/vizro/models/_components/form/slider.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
from typing import Any, Dict, List, Literal, Optional, Union
from typing import Dict, List, Literal, Optional

from dash import ClientsideFunction, Input, Output, State, clientside_callback, dcc, html

Expand Down Expand Up @@ -43,7 +43,7 @@ class Slider(VizroBaseModel):
min: Optional[float] = Field(None, description="Start value for slider.")
max: Optional[float] = Field(None, description="End value for slider.")
step: Optional[float] = Field(None, description="Step-size for marks on slider.")
marks: Optional[Dict[int, Union[str, Dict[str, Any]]]] = Field({}, description="Marks to be displayed on slider.")
marks: Optional[Dict[float, str]] = Field({}, description="Marks to be displayed on slider.")
huong-li-nguyen marked this conversation as resolved.
Show resolved Hide resolved
value: Optional[float] = Field(None, description="Default value for slider.")
title: str = Field("", description="Title to be displayed.")
actions: List[Action] = []
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -229,16 +229,20 @@ def test_validate_step_invalid(self):
"marks, expected",
[
({i: str(i) for i in range(0, 10, 5)}, {i: str(i) for i in range(0, 10, 5)}),
({15: 15, 25: 25}, {15.0: "15", 25.0: "25"}),
({"15": 15, "25": 25}, {15.0: "15", 25.0: "25"}),
({15: 15, 25: 25}, {15: "15", 25: "25"}), # all int
({15.5: 15.5, 25.5: 25.5}, {15.5: "15.5", 25.5: "25.5"}), # all floats
({15.0: 15, 25.5: 25.5}, {15: "15", 25.5: "25.5"}), # all floats, but convertible to int
huong-li-nguyen marked this conversation as resolved.
Show resolved Hide resolved
({"15": 15, "25": 25}, {15: "15", 25: "25"}), # all string
(None, None),
],
)
def test_valid_marks(self, marks, expected):
range_slider = vm.RangeSlider(min=0, max=10, marks=marks)

assert range_slider.marks == expected

if marks:
huong-li-nguyen marked this conversation as resolved.
Show resolved Hide resolved
assert all(type(key) is type(next(iter(expected.keys()))) for key in range_slider.marks)
huong-li-nguyen marked this conversation as resolved.
Show resolved Hide resolved

def test_invalid_marks(self):
with pytest.raises(ValidationError, match="2 validation errors for RangeSlider"):
vm.RangeSlider(min=1, max=10, marks={"start": 0, "end": 10})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,16 +120,20 @@ def test_valid_marks_with_step(self):
"marks, expected",
[
({i: str(i) for i in range(0, 10, 5)}, {i: str(i) for i in range(0, 10, 5)}),
({15: 15, 25: 25}, {15.0: "15", 25.0: "25"}),
({"15": 15, "25": 25}, {15.0: "15", 25.0: "25"}),
({15: 15, 25: 25}, {15: "15", 25: "25"}), # all int
({15.5: 15.5, 25.5: 25.5}, {15.5: "15.5", 25.5: "25.5"}), # all floats
({15.0: 15, 25.5: 25.5}, {15: "15", 25.5: "25.5"}), # all floats, but convertible to int
({"15": 15, "25": 25}, {15: "15", 25: "25"}), # all string
(None, None),
],
)
def test_valid_marks(self, marks, expected):
slider = vm.Slider(min=0, max=10, marks=marks)

assert slider.marks == expected

if marks:
assert all(type(key) is type(next(iter(expected.keys()))) for key in slider.marks)

def test_invalid_marks(self):
with pytest.raises(ValidationError, match="2 validation errors for Slider"):
vm.Slider(min=1, max=10, marks={"start": 0, "end": 10})
Expand Down
Loading