-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Conversation
@@ -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") |
There was a problem hiding this comment.
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.
There was a problem hiding this 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 💃
this PR will fix one part of #946 and added missing
python_requires
insetup.py