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

Add simplejson #3801

Merged
merged 14 commits into from
May 1, 2023
Merged

Add simplejson #3801

merged 14 commits into from
May 1, 2023

Conversation

bartbroere
Copy link
Contributor

@bartbroere bartbroere commented Apr 28, 2023

Add the simplejson package, so other packages can be more easily micropip installed if they happen to depend on this.

  • Add a CHANGELOG entry
  • Add / update tests
  • Add new / update outdated documentation

@lesteve
Copy link
Contributor

lesteve commented Apr 28, 2023

simplejson is pure Python right? So you should be able to install it with micropip.

so other packages can be more easily micropip installed if they happen to depend on this.

I don't quite understand what you mean by this, but maybe I am missing something. In principle await micropip.install('package-that-depends-on-simplejson') should work too, do you have an example of package where that does not work?

@bartbroere
Copy link
Contributor Author

No, it has some (optional) C extensions for speedups, and doesn't ship a pure Python wheel, so it can't be installed with micropip.

If we include it in Pyodide, we enable installing some libraries which have simplejson as a dependency.

@lesteve
Copy link
Contributor

lesteve commented Apr 28, 2023

OK I was not aware of this. For some reason await micropip.install('simplejson') is working fine in the stable console (the wheel it is using is https://files.pythonhosted.org/packages/56/40/c58cd470a57af4affa87da639a9d9d339a0d5898d98faa608ac43f3e191e/simplejson-3.19.1-py3-none-any.whl), but it is very likely not getting the optimised code written in C.

@bartbroere
Copy link
Contributor Author

Ah okay, not sure what error I got that led me to believe there wasn't a pure Python wheel. Sorry about that.

It still might be worthwhile to include this package with the speedups in Pyodide though.

The test assures the speedups are also there, so we don't install the pure Python wheel with extra steps.

@bartbroere
Copy link
Contributor Author

I found out what happened, the project that I micropip installed depended on an older version of simplejson, back when they didn't ship pure Python versions yet.

I'll make a PR to get that package up-to-date as well.

@hoodmane
Copy link
Member

Well it doesn't hurt to include a faster compiled version.

@bartbroere
Copy link
Contributor Author

Well it doesn't hurt to include a faster compiled version.

Alright! The test that imports simplejson._speedups should ensure that we don't end up with a pure Python version without realising it. (Which is possible because the setup.py seems to do pure Python as a fallback).

Let's hope it passes all CI checks this time.

[pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

Listen to pre commit bot
[pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci
.pre-commit-config.yaml Outdated Show resolved Hide resolved
@hoodmane
Copy link
Member

hoodmane commented May 1, 2023

Conveniently it doesn't depend on numpy so it isn't affected by #3816.

@hoodmane hoodmane merged commit 9aaa906 into pyodide:main May 1, 2023
@hoodmane
Copy link
Member

hoodmane commented May 1, 2023

Thanks @bartbroere!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants