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

Dash Pages bug fixes and enhancements for dash_pages param and layout module imports #2392

Merged
merged 28 commits into from
Mar 15, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
ab8de66
Fix bug where custom pages_folder wouldn't force use_pages=True
ned2 Jan 12, 2023
818d704
add two new pages folder tests and fix bug revealed
ned2 Jan 13, 2023
f6990c5
add more test_pages_folder_path_config and refactor to use parameteri…
ned2 Jan 14, 2023
0464477
fix bug where pages_folder can't take absolute paths and add tests
ned2 Jan 14, 2023
2070c4f
refactor pages_folder_config and add more tests
ned2 Jan 14, 2023
89d88b7
make pages_folder_config work with pathlib.Path inputs
ned2 Jan 14, 2023
fa1bf85
make pages_folder support pathlib.Path values
ned2 Jan 15, 2023
4e09114
add new test for pages_folder with absolute path
ned2 Jan 15, 2023
c833e99
add integration test for pages_folder with absolute path
ned2 Jan 15, 2023
d79c683
clear PAGES_REGISTRY after each pages integration test to avoid clash…
ned2 Jan 15, 2023
3426f62
move pages test dedicated folder
ned2 Jan 16, 2023
1e67a65
fix bug where pages_folder with underscores or upper case chars had w…
ned2 Jan 17, 2023
948a993
make Dash.pages_folder read-only
ned2 Jan 17, 2023
4f4d8fd
replace raised bare Exception with exception.PageError
ned2 Jan 17, 2023
fdeadde
add more infer_pages_path tests and improve variable naming
ned2 Jan 17, 2023
02c5ca5
fix borked long module names for pages files when pages_folder is abs…
ned2 Jan 22, 2023
2eedb00
Support page_folder 1 level below proj root and add clear_page_regist…
ned2 Jan 22, 2023
51404ed
Fix flaky tests broken by dirty _pages.CONFIG
ned2 Jan 22, 2023
21515af
Properly fix flaky test
ned2 Jan 23, 2023
900db46
Refactor test_pages_folder path tests to single parameterised test
ned2 Jan 23, 2023
1b3b6a4
add infer_path test case for nested directory paths
ned2 Jan 24, 2023
74252a8
Fix imports of modules by Dash Pages feature
ned2 Jan 27, 2023
7f9ed57
refactor _infer_module_name
ned2 Jan 27, 2023
03271db
add tests for _infer_module_name helper functions
ned2 Jan 27, 2023
11dbf86
Change default use_param=None and value of False now disables Pages e…
ned2 Mar 12, 2023
7d6d380
remove unused import
ned2 Mar 12, 2023
6fcf53b
Merge branch 'dev' into dash-pages-path-fixes
alexcjohnson Mar 15, 2023
04bbf0d
changelog for pages improvements
alexcjohnson Mar 15, 2023
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
8 changes: 7 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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),
Expand Down
27 changes: 10 additions & 17 deletions dash/_configs.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
57 changes: 44 additions & 13 deletions dash/_pages.py
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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:]
Expand Down
10 changes: 5 additions & 5 deletions dash/_validate.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
coerce_to_list,
clean_property_name,
)
from .exceptions import PageError


def validate_callback(outputs, inputs, state, extra_args, types):
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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

Expand Down
40 changes: 15 additions & 25 deletions dash/dash.py
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@

from . import _pages
from ._pages import (
_infer_module_name,
_parse_path_variables,
_parse_query_string,
)
Expand Down Expand Up @@ -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``
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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",
)
Expand All @@ -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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this breaks backward compatibility (combined with the change in dash/_configs.py) because effectively use_pages=True behavior is inferred (by checking page_folder instead of use_pages), while the user relies on the previous use_pages=False behavior.


# keep title as a class property for backwards compatibility
self.title = title
Expand Down Expand Up @@ -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("_")
]
Expand All @@ -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
Expand Down
26 changes: 26 additions & 0 deletions tests/conftest.py
Original file line number Diff line number Diff line change
@@ -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()
11 changes: 4 additions & 7 deletions tests/integration/multi_page/test_pages_layout.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down Expand Up @@ -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 = [
Expand Down Expand Up @@ -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 = [
Expand Down Expand Up @@ -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]
2 changes: 1 addition & 1 deletion tests/integration/multi_page/test_pages_order.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
Loading