Skip to content
This repository has been archived by the owner on Jun 4, 2024. It is now read-only.

add check that dash was correctly imported and exit with helpful message if failed #39

Merged
merged 3 commits into from
Mar 30, 2018

Conversation

ned2
Copy link
Contributor

@ned2 ned2 commented Mar 26, 2018

A common gotcha that people new to Dash encounter is naming their script 'dash.py' which then shadows the Dash package itself, causing unhappy times. This adds an explicit check that the dash.development attribute exists, which if it doesn't, it's likely this situation has happened, so a more helpful error message is printed to standard error, and the script exits.

This catches at least some of the times that this error occurs. If we also change the import order for example code from the Dash User guide to make from dash.dependencies import Input, State, Output come after after the import of dash_core_components this will then catch a lot more.

(This mirrors the same check added in plotly/dash-core-components#177)

@chriddyp
Copy link
Member

This is really, really great. Would you mind trying to add a test for this? i.e. create a file named dash.py, and assert that an error was raised?

Also, let's add this change here: https://github.com/plotly/dash-components-archetype/blob/master/init/%7B%7BpackageNameUnderscored%7D%7D/__init__.py, so that newly made packages get this fix as well.

@ned2
Copy link
Contributor Author

ned2 commented Mar 30, 2018

Yep, good idea regarding the test. I've just added one to a separate file so it can be easily copied to other dash component modules. It checks that sys.exit(1) occurred, which is how this PR behaves detects that Dash was not imported correctly.

@ned2
Copy link
Contributor Author

ned2 commented Mar 30, 2018

Just created a PR for the component archetype package over in plotly/dash-components-archetype#42

with self.assertRaises(SystemExit) as cm:
import dash_html_components

self.assertEqual(cm.exception.code, 1)
Copy link
Member

Choose a reason for hiding this comment

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

👌 perfect, thanks!

@chriddyp
Copy link
Member

this looks good to me. thanks @ned2 !

@chriddyp chriddyp merged commit 0d65ea6 into plotly:master Mar 30, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants