Merge pull request #2392 from ned2/dash-pages-path-fixes

Dash Pages bug fixes and enhancements for dash_pages param and layout module imports
plotly · Mar 15, 2023 · 82e4fb3 · 82e4fb3
2 parents cbef36d + 04bbf0d
commit 82e4fb3
Show file tree

Hide file tree

Showing 18 changed files with 313 additions and 79 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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)