Skip to content

Commit

Permalink
Introduce validator to raise ModuleNotFoundError at dashboard creation (
Browse files Browse the repository at this point in the history
#97)

Signed-off-by: Maximilian Schulz <[email protected]>
Co-authored-by: Antony Milne <[email protected]>
  • Loading branch information
maxschulz-COL and antonymilne authored Oct 6, 2023
1 parent 69abff5 commit 0cbe19f
Show file tree
Hide file tree
Showing 5 changed files with 64 additions and 30 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
<!--
A new scriv changelog fragment.
Uncomment the section that is right (remove the HTML comment wrapper).
-->

<!--
### Removed
- A bullet item for the Removed category.
-->

### Added

- Raise `ModuleNotFoundError` in case the `export_data` action is used with `file_format="xlsx"` and neither `openpyxl` nor `xlsxwriter` are installed ([#97](https://github.com/mckinsey/vizro/pull/97))

<!--
### Changed
- A bullet item for the Changed category.
-->
<!--
### Deprecated
- A bullet item for the Deprecated category.
-->
<!--
### Fixed
- A bullet item for the Fixed category.
-->
<!--
### Security
- A bullet item for the Security category.
-->
3 changes: 1 addition & 2 deletions vizro-core/src/vizro/actions/export_data_action.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,7 @@ def export_data(
writer = data_frames[target_id].to_csv
elif file_format == "xlsx":
writer = data_frames[target_id].to_excel
else:
raise ValueError(f'Unknown "file_format": {file_format}.' f' Known file formats: "csv", "xlsx".')
# Invalid file_format should be caught by Action validation

callback_outputs[f"download-dataframe_{target_id}"] = dcc.send_data_frame(
writer=writer, filename=f"{target_id}.{file_format}", index=False
Expand Down
21 changes: 20 additions & 1 deletion vizro-core/src/vizro/models/_action/_action.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import importlib.util
import logging
from typing import Any, Dict, List

from dash import Input, Output, State, callback, ctx
from pydantic import Field
from pydantic import Field, validator

import vizro.actions
from vizro.models import VizroBaseModel
Expand Down Expand Up @@ -35,6 +36,24 @@ class Action(VizroBaseModel):
regex="^[a-zA-Z0-9_]+[.][a-zA-Z_]+$",
)

# TODO: Problem: generic Action model shouldn't depend on details of particular actions like export_data.
# Possible solutions: make a generic mapping of action functions to validation functions or the imports they
# require, and make the code here look up the appropriate validation using the function as key
# This could then also involve other validations currently only carried out at run-time in pre-defined actions, such
# as e.g. checking if the correct arguments have been provided to the file_format in export_data.
@validator("function")
def validate_predefined_actions(cls, function):
if function._function.__name__ == "export_data":
file_format = function._arguments.get("file_format")
if file_format not in [None, "csv", "xlsx"]:
raise ValueError(f'Unknown "file_format": {file_format}.' f' Known file formats: "csv", "xlsx".')
if file_format == "xlsx":
if importlib.util.find_spec("openpyxl") is None and importlib.util.find_spec("xlsxwriter") is None:
raise ModuleNotFoundError(
"You must install either openpyxl or xlsxwriter to export to xlsx format."
)
return function

@_log_call
def build(self):
from vizro.actions._callback_mapping._get_action_callback_mapping import _get_action_callback_mapping
Expand Down
4 changes: 2 additions & 2 deletions vizro-core/src/vizro/models/types.py
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,8 @@ def __delitem__(self, arg_name: str):

@property
def _arguments(self):
# TODO: This is only used once in _get_parametrized_config and should be removed when that reference is
# removed.
# TODO: This is used twice: in _get_parametrized_config and in vm.Action and should be removed when those
# references are removed.
return self.__bound_arguments.arguments

@classmethod
Expand Down
25 changes: 0 additions & 25 deletions vizro-core/tests/unit/vizro/actions/test_export_data_action.py
Original file line number Diff line number Diff line change
Expand Up @@ -215,28 +215,3 @@ def test_multiple_targets_with_filter_and_filter_interaction(

assert result["download-dataframe_box_chart"]["filename"] == "box_chart.csv"
assert result["download-dataframe_box_chart"]["content"] == target_box_filtered_pop.to_csv(index=False)

@pytest.mark.usefixtures("managers_one_page_two_graphs_one_button")
@pytest.mark.parametrize(
"callback_context_export_data", [(["scatter_chart", "box_chart"], None, None)], indirect=True
)
def test_invalid_file_format(
self,
callback_context_export_data,
):
# Add action to relevant component
model_manager["button"].actions = [
vm.Action(
id="test_action",
function=export_data(
targets=["scatter_chart", "box_chart"],
file_format="invalid_file_format",
),
)
]

with pytest.raises(
ValueError, match='Unknown "file_format": invalid_file_format.' ' Known file formats: "csv", "xlsx".'
):
# Run action by picking the above added action function and executing it with ()
model_manager["test_action"].function()

0 comments on commit 0cbe19f

Please sign in to comment.