-
-
Notifications
You must be signed in to change notification settings - Fork 15
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
Updated HiPS catalog support #308
Conversation
See the code for detailed comments. This feels extremely gross but it seems to be working and I think it is actually a relatively good approach to getting this to work.
Thankfully, Python's async framework makes it pretty straightforward to wire up the message handling to support async operation. It only works in Jupyter with our massive hack, but that's not Python's fault.
This is what all of the async work is building up to. In order to populate HiPS catalogs on the fly, we *really* benefit from the async framework because we have to roundtrip with the WWT frontend repeatedly. This significantly builds on the work of @imbasimba in WorldWideTelescope#289, but builds on the work I've been doing to unify everything with the "research app" backend and these async foundations.
Codecov Report
@@ Coverage Diff @@
## master #308 +/- ##
==========================================
- Coverage 62.39% 59.38% -3.02%
==========================================
Files 24 24
Lines 2295 2489 +194
==========================================
+ Hits 1432 1478 +46
- Misses 863 1011 +148
Continue to review full report at Codecov.
|
YOLO. |
Interesting hack of the kernel :) To make it less fragile, we should probably control the allowed versions of the kernel a bit more. At least adding a minimum version for the ipykernel. It did not work with my previous ipykernel (5.5.4), but upgrading to the latest version (6.3.1) seems to solve the problem. So somewhere inbetween this hack starts to work, probably at 6.0. |
@imbasimba Yes, that's a good idea. I'm hoping that this approach won't turn into a maintenance nightmare, but that depends a bit on how the ipykernel code evolves :-/ |
Overview
This PR supersedes #289, building on the improved foundation that I've been working on. Namely:
The async support turns out to require an extremely gross hack of the Jupyter kernel framework but I believe that the approach is fundamentally OK, if hacky and fragile.
Checklist