diff --git a/CHANGELOG.md b/CHANGELOG.md index cd271a4381..cafbfc378d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,12 @@ This project adheres to [Semantic Versioning](https://semver.org/). ## Added +- [#2392](https://github.com/plotly/dash/pull/2392) Improved pages feature: + - Accept an absolute path or a `pathlib.path` for `pages_folder`, to match `assets_folder` + - Fix inferring `use_pages=True` when you supply a custom `pages_folder` + - Fix for `pages_folder` that includes special characters + - New test fixture `clear_pages_state` + - Make imported pages act more like regular Python modules - [#2068](https://github.com/plotly/dash/pull/2068) Added `refresh="callback-nav"` in `dcc.Location`. This allows for navigation without refreshing the page when url is updated in a callback. - [#2417](https://github.com/plotly/dash/pull/2417) Add wait_timeout property to customize the behavior of the default wait timeout used for by wait_for_page, fix [#1595](https://github.com/plotly/dash/issues/1595) - [#2417](https://github.com/plotly/dash/pull/2417) Add the element target text for wait_for_text* error message, fix [#945](https://github.com/plotly/dash/issues/945) @@ -32,7 +38,7 @@ This project adheres to [Semantic Versioning](https://semver.org/). ## Updated -- [#2241](https://github.com/plotly/dash/pull/2441) Update Plotly.js to v2.19.0 from v2.18.0. +- [#2241](https://github.com/plotly/dash/pull/2441) Update Plotly.js to v2.20.0 from v2.18.0. - Feature release [2.20.0](https://github.com/plotly/plotly.js/releases/tag/v2.20.0) adds `automargin` to the main plot title. - Feature release [2.19.0](https://github.com/plotly/plotly.js/releases/tag/v2.19.0) adds text labels to `layout.shapes`, and adds a `labelalias` property to replace specific axis tick labels. - Patch releases [2.18.1](https://github.com/plotly/plotly.js/releases/tag/v2.18.1), diff --git a/dash/_configs.py b/dash/_configs.py index 5c6631bc7d..cd6069d1ad 100644 --- a/dash/_configs.py +++ b/dash/_configs.py @@ -123,22 +123,15 @@ def pathname_configs( def pages_folder_config(name, pages_folder, use_pages): - if not use_pages: + if not pages_folder: return None - - pages_folder = pages_folder.lstrip("\\").lstrip("/") - pages_folder = None if pages_folder == "" else pages_folder - pages_folder_path = None - error_msg = f""" - A folder called {pages_folder} does not exist. - If a folder for pages is not required in your application, set `pages_folder=""` - For example, `app = Dash(__name__, pages_folder="")` - """ - - if pages_folder: - pages_folder_path = os.path.join( - flask.helpers.get_root_path(name), pages_folder - ) - if pages_folder and not os.path.isdir(pages_folder_path): - raise Exception(error_msg) + is_custom_folder = str(pages_folder) != "pages" + pages_folder_path = os.path.join(flask.helpers.get_root_path(name), pages_folder) + if (use_pages or is_custom_folder) and not os.path.isdir(pages_folder_path): + error_msg = f""" + A folder called `{pages_folder}` does not exist. If a folder for pages is not + required in your application, set `pages_folder=""`. For example: + `app = Dash(__name__, pages_folder="")` + """ + raise exceptions.InvalidConfig(error_msg) return pages_folder_path diff --git a/dash/_pages.py b/dash/_pages.py index 26083a2ef0..a4933dc3ec 100644 --- a/dash/_pages.py +++ b/dash/_pages.py @@ -1,10 +1,12 @@ +import collections import os -from os import listdir +import re +import sys +from fnmatch import fnmatch +from pathlib import Path from os.path import isfile, join -import collections from urllib.parse import parse_qs -from fnmatch import fnmatch -import re + import flask from . import _validate @@ -32,7 +34,7 @@ def _infer_image(module): if os.path.exists(assets_folder): files_in_assets = [ - f for f in listdir(assets_folder) if isfile(join(assets_folder, f)) + f for f in os.listdir(assets_folder) if isfile(join(assets_folder, f)) ] app_file = None logo_file = None @@ -57,29 +59,58 @@ def _infer_image(module): return logo_file -def _module_name_to_page_name(filename): - return filename.split(".")[-1].replace("_", " ").capitalize() +def _module_name_to_page_name(module_name): + return module_name.split(".")[-1].replace("_", " ").capitalize() -def _infer_path(filename, template): +def _infer_path(module_name, template): if template is None: if CONFIG.pages_folder: - pages_folder = CONFIG.pages_folder.replace("\\", "/").split("/")[-1] + pages_module = str(Path(CONFIG.pages_folder).name) path = ( - filename.replace("_", "-") + module_name.split(pages_module)[-1] + .replace("_", "-") .replace(".", "/") .lower() - .split(pages_folder)[-1] ) else: - path = filename.replace("_", "-").replace(".", "/").lower() + path = module_name.replace("_", "-").replace(".", "/").lower() else: - # replace the variables in the template with "none" to create a default path if no path is supplied + # replace the variables in the template with "none" to create a default path if + # no path is supplied path = re.sub("<.*?>", "none", template) path = "/" + path if not path.startswith("/") else path return path +def _module_name_is_package(module_name): + return ( + module_name in sys.modules + and Path(sys.modules[module_name].__file__).name == "__init__.py" + ) + + +def _path_to_module_name(path): + return str(path).replace(".py", "").strip(os.sep).replace(os.sep, ".") + + +def _infer_module_name(page_path): + relative_path = page_path.split(CONFIG.pages_folder)[-1] + module = _path_to_module_name(relative_path) + proj_root = flask.helpers.get_root_path(CONFIG.name) + if CONFIG.pages_folder.startswith(proj_root): + parent_path = CONFIG.pages_folder[len(proj_root) :] + else: + parent_path = CONFIG.pages_folder + parent_module = _path_to_module_name(parent_path) + + module_name = f"{parent_module}.{module}" + if _module_name_is_package(CONFIG.name): + # Only prefix with CONFIG.name when it's an imported package name + module_name = f"{CONFIG.name}.{module_name}" + return module_name + + def _parse_query_string(search): if search and len(search) > 0 and search[0] == "?": search = search[1:] diff --git a/dash/_validate.py b/dash/_validate.py index 24136aeed3..d1b94a284f 100644 --- a/dash/_validate.py +++ b/dash/_validate.py @@ -14,7 +14,6 @@ coerce_to_list, clean_property_name, ) -from .exceptions import PageError def validate_callback(outputs, inputs, state, extra_args, types): @@ -472,10 +471,12 @@ def validate_pages_layout(module, page): def validate_use_pages(config): if not config.get("assets_folder", None): - raise PageError("`dash.register_page()` must be called after app instantiation") + raise exceptions.PageError( + "`dash.register_page()` must be called after app instantiation" + ) if flask.has_request_context(): - raise PageError( + raise exceptions.PageError( """ dash.register_page() can’t be called within a callback as it updates dash.page_registry, which is a global variable. For more details, see https://dash.plotly.com/sharing-data-between-callbacks#why-global-variables-will-break-your-app @@ -485,7 +486,7 @@ def validate_use_pages(config): def validate_module_name(module): if not isinstance(module, str): - raise Exception( + raise exceptions.PageError( "The first attribute of dash.register_page() must be a string or '__name__'" ) return module @@ -526,7 +527,6 @@ def validate_long_callbacks(callback_map): def validate_duplicate_output( output, prevent_initial_call, config_prevent_initial_call ): - if "initial_duplicate" in (prevent_initial_call, config_prevent_initial_call): return diff --git a/dash/dash.py b/dash/dash.py index 5a2d23912b..b70709b306 100644 --- a/dash/dash.py +++ b/dash/dash.py @@ -64,6 +64,7 @@ from . import _pages from ._pages import ( + _infer_module_name, _parse_path_variables, _parse_query_string, ) @@ -204,9 +205,10 @@ class Dash: for pages of a multi-page app. Default ``'pages'``. :type pages_folder: string - :param use_pages: Default False, or True if you set a non-default ``pages_folder``. - When True, the ``pages`` feature for multi-page apps is enabled. - :type pages: boolean + :param use_pages: When True, the ``pages`` feature for multi-page apps is + enabled. If you set a non-default ``pages_folder`` this will be inferred + to be True. Default `None`. + :type use_pages: boolean :param assets_url_path: The local urls for assets will be: ``requests_pathname_prefix + assets_url_path + '/' + asset_path`` @@ -340,7 +342,7 @@ def __init__( # pylint: disable=too-many-statements server=True, assets_folder="assets", pages_folder="pages", - use_pages=False, + use_pages=None, assets_url_path="assets", assets_ignore="", assets_external_path=None, @@ -425,6 +427,7 @@ def __init__( # pylint: disable=too-many-statements "eager_loading", "serve_locally", "compress", + "pages_folder", ], "Read-only: can only be set in the Dash constructor", ) @@ -436,8 +439,8 @@ def __init__( # pylint: disable=too-many-statements _get_paths.CONFIG = self.config _pages.CONFIG = self.config - self.pages_folder = pages_folder - self.use_pages = True if pages_folder != "pages" else use_pages + self.pages_folder = str(pages_folder) + self.use_pages = (pages_folder != "pages") if use_pages is None else use_pages # keep title as a class property for backwards compatibility self.title = title @@ -1986,9 +1989,7 @@ def verify_url_part(served_part, url_part, part_name): self.server.run(host=host, port=port, debug=debug, **flask_run_options) def _import_layouts_from_pages(self): - walk_dir = self.config.pages_folder - - for (root, dirs, files) in os.walk(walk_dir): + for root, dirs, files in os.walk(self.config.pages_folder): dirs[:] = [ d for d in dirs if not d.startswith(".") and not d.startswith("_") ] @@ -1999,28 +2000,17 @@ def _import_layouts_from_pages(self): or not file.endswith(".py") ): continue - with open(os.path.join(root, file), encoding="utf-8") as f: + page_path = os.path.join(root, file) + with open(page_path, encoding="utf-8") as f: content = f.read() if "register_page" not in content: continue - page_filename = os.path.join(root, file).replace("\\", "/") - _, _, page_filename = page_filename.partition( - walk_dir.replace("\\", "/") + "/" - ) - page_filename = page_filename.replace(".py", "").replace("/", ".") - - pages_folder = ( - self.pages_folder.replace("\\", "/").lstrip("/").replace("/", ".") - ) - - module_name = ".".join([pages_folder, page_filename]) - - spec = importlib.util.spec_from_file_location( - module_name, os.path.join(root, file) - ) + module_name = _infer_module_name(page_path) + spec = importlib.util.spec_from_file_location(module_name, page_path) page_module = importlib.util.module_from_spec(spec) spec.loader.exec_module(page_module) + sys.modules[module_name] = page_module if ( module_name in _pages.PAGE_REGISTRY diff --git a/tests/conftest.py b/tests/conftest.py new file mode 100644 index 0000000000..c5801b3ddc --- /dev/null +++ b/tests/conftest.py @@ -0,0 +1,26 @@ +import os + +import pytest +import dash +from dash._configs import DASH_ENV_VARS + + +@pytest.fixture +def empty_environ(): + for k in DASH_ENV_VARS.keys(): + if k in os.environ: + os.environ.pop(k) + + +@pytest.fixture +def clear_pages_state(): + init_pages_state() + yield + init_pages_state() + + +def init_pages_state(): + """Clear all global state that is used by pages feature.""" + dash._pages.PAGE_REGISTRY.clear() + dash._pages.CONFIG.clear() + dash._pages.CONFIG.__dict__.clear() diff --git a/tests/integration/multi_page/test_pages_layout.py b/tests/integration/multi_page/test_pages_layout.py index d5abb51ff7..32573aaf36 100644 --- a/tests/integration/multi_page/test_pages_layout.py +++ b/tests/integration/multi_page/test_pages_layout.py @@ -48,7 +48,7 @@ def get_app(path1="/", path2="/layout2"): return app -def test_pala001_layout(dash_duo): +def test_pala001_layout(dash_duo, clear_pages_state): app = get_app() dash_duo.start_server(app) @@ -113,7 +113,7 @@ def check_metas(dash_duo, metas): assert meta[i].get_attribute("content") == metas[i]["content"] -def test_pala002_meta_tags_default(dash_duo): +def test_pala002_meta_tags_default(dash_duo, clear_pages_state): dash_duo.start_server(get_app(path1="/layout1", path2="/")) # These are the inferred defaults if description, title, image are not supplied metas_layout2 = [ @@ -141,7 +141,7 @@ def test_pala002_meta_tags_default(dash_duo): check_metas(dash_duo, metas_layout2) -def test_pala003_meta_tags_custom(dash_duo): +def test_pala003_meta_tags_custom(dash_duo, clear_pages_state): dash_duo.start_server(get_app()) # In the "multi_layout1" module, the description, title, image are supplied metas_layout1 = [ @@ -172,13 +172,10 @@ def test_pala003_meta_tags_custom(dash_duo): check_metas(dash_duo, metas_layout1) -def test_pala004_no_layout_exception(): +def test_pala004_no_layout_exception(clear_pages_state): error_msg = 'No layout found in module pages_error.no_layout_page\nA variable or a function named "layout" is required.' with pytest.raises(NoLayoutException) as err: Dash(__name__, use_pages=True, pages_folder="pages_error") - # clean up after this test, so the broken entry doesn't affect other pages tests - del dash.page_registry["pages_error.no_layout_page"] - assert error_msg in err.value.args[0] diff --git a/tests/integration/multi_page/test_pages_order.py b/tests/integration/multi_page/test_pages_order.py index 57d487f437..00c05f6b1e 100644 --- a/tests/integration/multi_page/test_pages_order.py +++ b/tests/integration/multi_page/test_pages_order.py @@ -2,7 +2,7 @@ from dash import Dash, dcc, html -def test_paor001_order(dash_duo): +def test_paor001_order(dash_duo, clear_pages_state): app = Dash(__name__, use_pages=True, suppress_callback_exceptions=True) diff --git a/tests/integration/multi_page/test_pages_relative_path.py b/tests/integration/multi_page/test_pages_relative_path.py index 2cbc63402a..3d6345a4a8 100644 --- a/tests/integration/multi_page/test_pages_relative_path.py +++ b/tests/integration/multi_page/test_pages_relative_path.py @@ -1,3 +1,5 @@ +from pathlib import Path + import dash from dash import Dash, dcc, html @@ -43,8 +45,7 @@ def get_app(app): return app -def test_pare001_relative_path(dash_duo): - +def test_pare001_relative_path(dash_duo, clear_pages_state): dash_duo.start_server(get_app(Dash(__name__, use_pages=True))) for page in dash.page_registry.values(): dash_duo.find_element("#" + page["id"]).click() @@ -54,7 +55,9 @@ def test_pare001_relative_path(dash_duo): assert dash_duo.get_logs() == [], "browser console should contain no error" -def test_pare002_relative_path_with_url_base_pathname(dash_br, dash_thread_server): +def test_pare002_relative_path_with_url_base_pathname( + dash_br, dash_thread_server, clear_pages_state +): dash_thread_server( get_app(Dash(__name__, use_pages=True, url_base_pathname="/app1/")) ) @@ -66,3 +69,16 @@ def test_pare002_relative_path_with_url_base_pathname(dash_br, dash_thread_serve assert dash_br.driver.title == page["title"], "check that page title updates" assert dash_br.get_logs() == [], "browser console should contain no error" + + +def test_pare003_absolute_path(dash_duo, clear_pages_state): + pages_folder = Path(__file__).parent / "pages" + dash_duo.start_server( + get_app(Dash(__name__, use_pages=True, pages_folder=pages_folder)) + ) + for page in dash.page_registry.values(): + dash_duo.find_element("#" + page["id"]).click() + dash_duo.wait_for_text_to_equal("#text_" + page["id"], "text for " + page["id"]) + assert dash_duo.driver.title == page["title"], "check that page title updates" + + assert dash_duo.get_logs() == [], "browser console should contain no error" diff --git a/tests/unit/pages/__init__.py b/tests/unit/pages/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/unit/pages/custom_pages/__init__.py b/tests/unit/pages/custom_pages/__init__.py new file mode 100644 index 0000000000..d1e20ba325 --- /dev/null +++ b/tests/unit/pages/custom_pages/__init__.py @@ -0,0 +1,2 @@ +# this directory is used for testing custom values of `pages_folder` param that +# need to exist as actual paths. diff --git a/tests/unit/pages/custom_pages/page.py b/tests/unit/pages/custom_pages/page.py new file mode 100644 index 0000000000..87eb505517 --- /dev/null +++ b/tests/unit/pages/custom_pages/page.py @@ -0,0 +1,5 @@ +import dash + +dash.register_page(__name__) + +layout = dash.html.Div("") diff --git a/tests/unit/pages/sub_dir/__init__.py b/tests/unit/pages/sub_dir/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/unit/pages/sub_dir/custom_pages/__init__.py b/tests/unit/pages/sub_dir/custom_pages/__init__.py new file mode 100644 index 0000000000..d1e20ba325 --- /dev/null +++ b/tests/unit/pages/sub_dir/custom_pages/__init__.py @@ -0,0 +1,2 @@ +# this directory is used for testing custom values of `pages_folder` param that +# need to exist as actual paths. diff --git a/tests/unit/pages/sub_dir/custom_pages/page.py b/tests/unit/pages/sub_dir/custom_pages/page.py new file mode 100644 index 0000000000..87eb505517 --- /dev/null +++ b/tests/unit/pages/sub_dir/custom_pages/page.py @@ -0,0 +1,5 @@ +import dash + +dash.register_page(__name__) + +layout = dash.html.Div("") diff --git a/tests/unit/pages/test_pages.py b/tests/unit/pages/test_pages.py new file mode 100644 index 0000000000..df943c8f9b --- /dev/null +++ b/tests/unit/pages/test_pages.py @@ -0,0 +1,78 @@ +from pathlib import Path + +import pytest +import dash +from dash import Dash, _pages +from mock import patch + + +THIS_DIR = Path(__file__).parent + + +@pytest.mark.parametrize( + "module_name, template, pages_folder, expected", + [ + ("pages.page1", None, str(THIS_DIR / "pages"), "/page1"), + ("Pages.page1", None, str(THIS_DIR / "Pages"), "/page1"), + ("custom_pages.page1", None, str(THIS_DIR / "custom_pages"), "/page1"), + ("custom.pages.page1", None, str(THIS_DIR / "custom.pages"), "/page1"), + ("custom.pages.page1", None, str(THIS_DIR / "custom" / "pages"), "/page1"), + ( + "custom_pages.chapter_1.page_1", + None, + str(THIS_DIR / "custom_pages"), + "/chapter-1/page-1", + ), + # can this even be called with CONFIG.pages_folder set to None? + ("dir.my_page", None, None, "/dir/my-page"), + # is this behaviour right? why is filename ignored when template has a value? + ("pages.page1", "/items/", str(THIS_DIR / "pages"), "/items/none"), + ], +) +def test_infer_path(clear_pages_state, module_name, template, pages_folder, expected): + with patch.dict(_pages.CONFIG, {"pages_folder": pages_folder}, clear=True): + result = _pages._infer_path(module_name, template) + assert result == expected + + +@pytest.mark.parametrize( + "module_name, expected", + [ + (__name__, False), + (__package__, True), + ], +) +def test_module_name_is_package(module_name, expected): + assert _pages._module_name_is_package(module_name) == expected + + +@pytest.mark.parametrize( + "path, expected", + [ + ("/page.py", "page"), + ("/pages/page.py", "pages.page"), + ("/pages", "pages"), + ("/sub_dir/pages", "sub_dir.pages"), + ], +) +def test_path_to_module_name(path, expected): + assert _pages._path_to_module_name(path) == expected + + +@pytest.mark.parametrize( + "name, pages_folder, expected_module_name", + [ + (__name__, "custom_pages", "custom_pages.page"), + (__name__, "sub_dir/custom_pages", "sub_dir.custom_pages.page"), + (__name__, str(THIS_DIR / "custom_pages"), "custom_pages.page"), + (__package__, "custom_pages", "pages.custom_pages.page"), + ], +) +def test_import_layouts_from_pages( + clear_pages_state, name, pages_folder, expected_module_name +): + _ = Dash(name, use_pages=True, pages_folder=pages_folder) + assert len(dash.page_registry) == 1 + + page_entry = list(dash.page_registry.values())[0] + assert page_entry["module"] == expected_module_name diff --git a/tests/unit/pages/test_pages_config.py b/tests/unit/pages/test_pages_config.py new file mode 100644 index 0000000000..958580422d --- /dev/null +++ b/tests/unit/pages/test_pages_config.py @@ -0,0 +1,90 @@ +from contextlib import contextmanager +from pathlib import Path + +import pytest + +from dash import Dash, exceptions as _exc +from dash._configs import pages_folder_config + + +THIS_DIR = Path(__file__).parent + + +@contextmanager +def does_not_raise(): + """Context manager for testing no exception is raised""" + yield + + +@pytest.mark.parametrize( + "pages_folder, use_pages, expected_pages_folder_path", + [ + ("", False, None), + (None, False, None), + ("pages", False, str(THIS_DIR / "pages")), + (Path("pages"), False, str(THIS_DIR / "pages")), + ("custom_pages", True, str(THIS_DIR / "custom_pages")), + ("custom_pages", False, str(THIS_DIR / "custom_pages")), + ( + str(THIS_DIR / "custom_pages"), + True, + str(THIS_DIR / "custom_pages"), + ), + ( + str(THIS_DIR / "custom_pages"), + False, + str(THIS_DIR / "custom_pages"), + ), + ( + THIS_DIR / "custom_pages", + False, + str(THIS_DIR / "custom_pages"), + ), + ("sub_dir/custom_pages", True, str(THIS_DIR / "sub_dir" / "custom_pages")), + ], +) +def test_pages_folder_config( + empty_environ, pages_folder, use_pages, expected_pages_folder_path +): + pages_folder_path = pages_folder_config(__name__, pages_folder, use_pages) + assert pages_folder_path == expected_pages_folder_path + + +@pytest.mark.parametrize( + "pages_folder, use_pages, expectation", + [ + ("pages", False, does_not_raise()), + ("pages", True, pytest.raises(_exc.InvalidConfig)), + ("does_not_exist", True, pytest.raises(_exc.InvalidConfig)), + ("does_not_exist", False, pytest.raises(_exc.InvalidConfig)), + ], +) +def test_pages_missing_path_config(empty_environ, pages_folder, use_pages, expectation): + with expectation: + _ = pages_folder_config(__name__, pages_folder, use_pages) + + +@pytest.mark.parametrize( + "use_pages, pages_folder", + [ + (True, "custom_pages"), + (True, Path("custom_pages")), + (True, str(THIS_DIR / "custom_pages")), + (True, THIS_DIR / "custom_pages"), + (True, str(THIS_DIR / "sub_dir" / "custom_pages")), + (True, THIS_DIR / "sub_dir" / "custom_pages"), + (None, "custom_pages"), + (None, "pages"), + (False, "custom_pages"), + ], +) +def test_pages_folder_app_config( + empty_environ, clear_pages_state, use_pages, pages_folder +): + app = Dash(__name__, use_pages=use_pages, pages_folder=pages_folder) + if use_pages is None: + expected_use_pages = bool(pages_folder != "pages") + elif use_pages in (True, False): + expected_use_pages = use_pages + assert app.use_pages == expected_use_pages + assert app.pages_folder == str(pages_folder) diff --git a/tests/unit/test_configs.py b/tests/unit/test_configs.py index d5e0c1c256..ca026d2211 100644 --- a/tests/unit/test_configs.py +++ b/tests/unit/test_configs.py @@ -25,13 +25,6 @@ ) -@pytest.fixture -def empty_environ(): - for k in DASH_ENV_VARS.keys(): - if k in os.environ: - os.environ.pop(k) - - def test_dash_env_vars(empty_environ): assert {None} == { val for _, val in DASH_ENV_VARS.items()