-
Notifications
You must be signed in to change notification settings - Fork 989
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
Add warning for problematic libraries #2151
Conversation
# Need to see why this test raises forking issues when ran as a suite | ||
@skip |
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.
Essentially the notebook_launcher
doesn't like using async subprocesses when doing things, so executing it fully as a non-async subprocess makes this fully work reliably.
The documentation is not available anymore as the PR was closed or merged. |
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.
Good idea to make this an explicit error, given how many users stumble upon this. I only have some smaller comments.
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.
Thanks for addressing the points I raised. I only have a small comment about a type annotation, feel free to merge.
What does this PR do?
Since this is a very common issue, adds a check in
notebook_launcher
for specific libraries that are known to cause issues (such as starting CUDA forks on import).Fixes #2148
Before submitting
Pull Request section?
to it if that's the case.
documentation guidelines, and
here are tips on formatting docstrings.
Who can review?
Anyone in the community is free to review the PR once the tests have passed. Feel free to tag
members/contributors who may be interested in your PR.
@BenjaminBossan @SunMarc