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

Update vendored double-conversion to 3.2.1 #570

Merged
merged 2 commits into from Nov 3, 2022

Conversation

joemarshall
Copy link
Contributor

Ultrajson doesn't build on webassembly (e.g. pyodide) because the version of double-conversion used is too old. This updates it to a newer version which supports webassembly.

@codecov-commenter
Copy link

Codecov Report

Merging #570 (ffe12ed) into main (13da58c) will not change coverage.
The diff coverage is n/a.

❗ Current head ffe12ed differs from pull request most recent head 8386a41. Consider uploading reports for the commit 8386a41 to get more accurate results

@@           Coverage Diff           @@
##             main     #570   +/-   ##
=======================================
  Coverage   91.49%   91.49%           
=======================================
  Files           6        6           
  Lines        1905     1905           
=======================================
  Hits         1743     1743           
  Misses        162      162           

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@joemarshall
Copy link
Contributor Author

About this - It needs webassembly tests adding, I can do this once a PR to pytest-pyodide (pyodide/pytest-pyodide#59 )is added to make it possibly to skip non-pyodide tests

@bwoodsend
Copy link
Collaborator

What would make a test non-pyodide? Any reason we can't just add a few

@pytest.mark.skipif(platform.machine().startswith("wasm"), "Not supported on pyodide")

statements instead (assuming there's not a better way to detect if running on pyodide)?

@bwoodsend bwoodsend added the changelog: Added For new features label Nov 2, 2022
@bwoodsend bwoodsend changed the title update double-conversion to 3.2.1 for wasm support Support compiling to WASM (by updating vendored double-conversion to 3.2.1) Nov 2, 2022
@joemarshall
Copy link
Contributor Author

@bwoodsend pyodide uses a custom pytest plugin which assumes you're writing tests to run things in pyodide. It means you can do things like spin up servers for pyodide code in a browser to talk to. But right now there's no way to just run normal pytest tests on pyodide. It would need to spin up a browser+pyodide for each test or for each module of tests or something, and then run the tests in there automatically.

At the moment if you want pyodide tests, you have to write a custom test_pyodide.py for your project which wraps the project tests and calls them in pyodide.

@bwoodsend
Copy link
Collaborator

Okay, fair enough. I'll merge this now since it's only updating double-conversion (which we really ought to be more proactive in keeping up to date anyway) and keep any mention of supporting WASM out of the changelog until the tests go in.

@bwoodsend bwoodsend changed the title Support compiling to WASM (by updating vendored double-conversion to 3.2.1) Update vendored double-conversion to 3.2.1 Nov 3, 2022
@bwoodsend bwoodsend merged commit 2907fde into ultrajson:main Nov 3, 2022
@joemarshall
Copy link
Contributor Author

@bwoodsend Any chance of a release to pypi to get this version onto pypi and save me build pain...

@bwoodsend
Copy link
Collaborator

Release button has been pressed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog: Added For new features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants