-
Notifications
You must be signed in to change notification settings - Fork 391
Replace urllib.request with urllib3 to support WebAssembly #2515
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
base: main
Are you sure you want to change the base?
Conversation
|
|
|
Is this PR still active? I am running into the same problem and would love to see a solution. |
|
It is still open, but needs a champion that is interested in merging it in. I'm not in favor of it because it adds a new dependency that seems somewhat unncessary except for a WASM environment, but I won't block if someone else thinks this is the way to go. See the comment here and the thread for workarounds/suggestions on how to approach this: #2514 (comment) |
|
The workaround is reasonably easy and works for me. Thanks for pointing to it. On the other hand, "except for a WASM environment" seems like a very good reason to me. Especially considering that WASM is a pretty universal standard these days rapidly gaining popularity through platforms such as JupyterLite. But I'm not neutral. |
dopplershift
left a comment
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.
I've come around in favor of this approach for a few reasons:
- I'm interested in WASM
- We're already depending on requests conditionally through owslib
- urllib3 being a dependency of requests makes it 99% likely it's already installed in someone's environment and not a risky add.
I'll wait to merge a bit for any impassioned counterpoints, but IMO this is a reasonable merge.
|
Still needs the CLA to be signed, no opinion on the dependency myself. |
As discussed in #2514,
urllib.requestis not supported in WebAssembly. This replacesurllib.requestwhere needed withurllib3which does work with WebAssembly. I've tried to keep the code changes to a minimum and to reuse existing semantic patterns.The linter did introduce new lines in one of the imports. I found it odd but left it in place.
Rationale
This enables the use of Cartopy is Pyodide and JupyterLite. Without it, downloading of shape files, etc., fail. See #2514.
Implications
I've run the unit tests and nothing new fails. It's also been tested in JupyterLite and works. There is a chance that the download code path hasn't been fully tested and something may still need to be changed to support
urllib3.This also introduces a new dependency, even though
urllib3is widely available. It is also an existing package in Pyodide.