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

Fix #99: "RuntimeError: This event loop is already running" in colab and notebook. Re-PR of #100 #130

Merged
merged 4 commits into from Aug 25, 2021

Conversation

aaraney
Copy link
Member

@aaraney aaraney commented Aug 24, 2021

This is a resubmission of #100 with minor changes to the unit test added.
Fixes #99.

Changes

  • If event loop is already running and RestClient is instantiated, try to import nest_asyncio and patch with nest_asyncio.apply(). Raise ModuleNotFoundError if nest_asyncio not installed.

Testing

  1. Test added to verify ModuleNotFoundError is raised. If nest_asyncio is installed, test fails.

Checklist

  • PR has an informative and human-readable title
  • PR is well outlined and documented. See #12 for an example
  • Changes are limited to a single goal (no scope creep)
  • Code can be automatically merged (no conflicts)
  • Code follows project standards (see CONTRIBUTING.md)
  • Passes all existing automated tests
  • Any change in functionality is tested
  • New functions are documented (with a description, list of inputs, and expected output) using numpy docstring formatting
  • Placeholder code is flagged / future todos are captured in comments
  • Reviewers requested with the Reviewers tool ➡️

@jarq6c
Copy link
Collaborator

jarq6c commented Aug 25, 2021

@aaraney Have you confirmed this change fixes the original google collab issue? Our other software devs may have an opinion on the general pattern, but nothing appears particularly obscene about this contribution to me.

@aaraney
Copy link
Member Author

aaraney commented Aug 25, 2021

@aaraney Have you confirmed this change fixes the original google collab issue? Our other software devs may have an opinion on the general pattern, but nothing appears particularly obscene about this contribution to me.

Yes. Here is a colab notebook that verifies this PR resolves the previous issue.

@jarq6c jarq6c merged commit 908e000 into NOAA-OWP:main Aug 25, 2021
@jarq6c
Copy link
Collaborator

jarq6c commented Aug 25, 2021

Thanks for the fix!

@aaraney
Copy link
Member Author

aaraney commented Aug 25, 2021

For sure!

@aaraney aaraney deleted the re-fix-runtime-error-in-restclient branch May 19, 2023 01:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants