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

🔇 remove the warning as it might confuse non dash usage #1015

Merged
merged 9 commits into from
Nov 22, 2019

Conversation

byronz
Copy link
Contributor

@byronz byronz commented Nov 15, 2019

this PR will fix one part of #946 and added missing python_requires in setup.py

@@ -14,7 +11,7 @@
from dash.testing.browser import Browser
from dash.testing.composite import DashComposite, DashRComposite
except ImportError:
warnings.warn("run `pip install dash[testing]` if you need dash.testing")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Wouldn't we also need to put all the below definitions into the try block, so that without the necessary imports it won't try to define our fixtures?

That said, this still doesn't seem like it completely satisfies the request of #946 - this takes care of the "annoys with the warning" part, but not the "increases the time of test suite by unnecessary import of the module" part. As I said in the issue, it's not clear to me how one can resolve that part, but the ideal seems to me determining whether setup.py was called with [testing] or not - and only include "pytest11": ["dash = dash.testing.plugin"] if yes.

setup.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@alexcjohnson alexcjohnson left a comment

Choose a reason for hiding this comment

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

Alright, this is a definite improvement - as long as we leave #946 open and keep our eyes out for a more complete solution, I'm happy to add this piece 💃

@alexcjohnson alexcjohnson merged commit 53d8fcc into dev Nov 22, 2019
@alexcjohnson alexcjohnson deleted the 946-testing-entrypoint branch November 22, 2019 18:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants