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

[ned2]

Dash Pages Improvements

Fixes and enhancements to dash_pages param

• pages_folder param now accepts an absolute path (previously would throw an exception), bringing it in line with how assets_folder behaves
• pages_folder param now accepts a pathlib.Path instance (previously would throw an exception), bringing it in line with how assets_folder behaves
• fixed a bug where pages_folder being set to to custom path (ie not the default value "path") did not force use_pages to be set to True (as indicated should occur in the Dash doc string)
• fixed a bug where pages_folder values containing _, ., or uppercase chars resulted in incorrectly derived default path values in dash.page_registry (eg my_pages/page.py would result in a path of /my-pages/page rather than /page)
• set app.config.pages_folder to be read-only, since this param should only be set in the Dash.__init__()
• replace a bare Exception in dash._validate and one in dash._pages with appropriate exception subclasses
• create fixture clear_pages_state to clear to reset all global Pages state before and after running of all tests that modify that state. this is needed to ensure that tests for Pages features are not broken by state set by other tests (and they don't break subsequent tests)

Dash Community forum thread where the question was asked if there was any reason why pages_folder couldn't support absolute paths: https://community.plotly.com/t/set-pages-folder-to-an-absolute-path/71347

Fixes to imports performed by Dash Pages

The Python modules found inside the dash_pages directory that are programatically imported for loading layouts had two problems that this PR also fixes:

• Add imported auto Pages modules to sys.modules; fixes [BUG] page modules not imported correctly #2312
• When running inside a package and the name param of the Dash instance is set to __package__, the module name of auto imported Pages modules will now be prefixed with the package name. ie myproj/pages/page1.py -> myproj.pages.page1 (previously would have been pages.page1). Fixes [BUG] Dash Pages does not support relative package imports within page modules #2263

[ned2]

hey @alexcjohnson, for some context, this started off from a discussion I had with @ann-marie-ward about supporting absolute paths for pages_folder, prompted by this community forum thread. While working on this I discovered some other bugs and low hanging improvements surrounding pages_folder configuration as described above, hence why this PR is a little on the chonkier side.

While working on this, I've also come up with what I think is a fairly clean solution to both #2263 and #2312, which doesn't involve the heavy-handed approach of clearing any global state, as I proposed in #2271 (but does assume the existence of the clear_pages_state fixture introduced in this PR).

Would it make sense to bundle these improvements into this PR since they involve the same parts of the codebase, or should I hold of for a separate PR? (Assuming you're on board with this one here in the first place!)

[alexcjohnson]

@ned2 this looks great! Really appreciate all the work you put into tests here as well.

While working on this, I've also come up with what I think is a fairly clean solution to both #2263 and #2312, which doesn't involve the heavy-handed approach of clearing any global state, as I proposed in #2271 (but does assume the existence of the clear_pages_state fixture introduced in this PR).

Meaning you'd use the clear_pages_state not in a test context but in a real app? That strikes me as funny, but I'm open to seeing what you have in mind, and sure, it can be part of this same PR.

[ned2]

OK @alexcjohnson, have added in the fixes for Dash Pages imports as described in the updated PR description. Ready for review!

Meaning you'd use the clear_pages_state not in a test context but in a real app? That strikes me as funny, but I'm open to seeing what you have in mind, and sure, it can be part of this same PR.

Oh no, yeah that would be weird. I'm only using clear_pages_state within the tests, for any test using Dash Pages. The only reason I was clearing Dash internal state in that other PR was to keep the tests suite happy, where multiple invocations of Dash within the same process was causing issues. You can see that in one case this was already being managed by reaching directly into the pages registry and manipulating it after a test ran. This problem only became worse with all the new tests and newly prefixed module names.

Using the new fixture to clear state before and after the test is a little less heavy handed than actually clearing Dash state. (That said, I wonder if there is still a broader argument to clear Dash's global state to prevent these kinds if possible gotchas)

btw, looks like something flaky is going on with the tests. Chrome is failing to start up intermittently on some tests in this and the previous run a push triggered.

[ned2]

heya @alexcjohnson, just bumping this. would be great to get a review while it's still in my head, if you have capacity sometime soon :)

[alexcjohnson]

Not new in this PR but since you're fixing some related edge cases... currently if you call:

app = Dash(pages_folder="blah", use_pages=False)

the custom pages_folder wins and enables pages even though you explicitly said use_pages=False. I'm not sure why you'd do that (maybe to disable pages temporarily for debugging?), but it feels backward to me. Maybe we can switch use_pages to have a default of None, then this logic here can become something like:

self.use_pages = (pages_folder != "pages") if use_pages is None else use_pages

Or maybe even centralize all of this logic (after changing the default use_pages=None) in _configs.py, then these lines can just draw from config:

self.pages_folder = config.pages_folder
self.use_pages = bool(config.pages_folder)

[ned2]

ahh yep, agreed, would be good to squeeze that in. I'll get onto it!

[ned2]

Just a heads up @alexcjohnson that this suggestion didn't end up making it in, perhaps could be worth creating an issue for it?

[alexcjohnson]

You're adding a bunch of __init__.py files to the tests in this PR - are these now required for pages to work or did you just add them for best practice?

[ned2]

In general, no, __init__.py are not required for Dash pages to work. However Dash Pages now detects when discovered page modules happen to be inside of a package so this it can add the package name as a prefix to the module path. See _infer_module_name. Note this relies on configuring Dash like so: Dash(name=__package__)

So in this particular case the __init__.py needs to be added to make the test app a package for the tests to work. The other instances I just added them in for best-practice.

[alexcjohnson]

@ned2 thanks for the nudge. Just two small questions, then we should be good to go! The first about explicit use_pages=False is nonblocking but feels like the right time to address it while we're tightening up the rest of this logic. The second I just want to make sure we aren't accidentally breaking existing apps - I don't see why we would be but worth confirming.

[alexcjohnson]

💃 Thanks @ned2 - this will go in v2.9 🎉

[miraculixx]

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.