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

Remove ujson dependency from setup.py? #31

Closed
bmschmidt opened this issue Dec 30, 2019 · 3 comments
Closed

Remove ujson dependency from setup.py? #31

bmschmidt opened this issue Dec 30, 2019 · 3 comments

Comments

@bmschmidt
Copy link
Contributor

Trying to install on a wonky machine, I found I had to manually remove the entry for ujson from setup.py because I was having trouble compiling it.

In general, ujson is a dead package; it might be worth switching to orjson for speed. But since there's already a fallback to standard json in the code here, I don't think it would hurt to just remove it from the setup?

@organisciak
Copy link
Collaborator

I'll update this in the Massive Texts fork, since I'll be looking to merge that here in the coming month or so. orjson looks good - the built-in library was a bottleneck. The side benefit is that I can stop maintaining a conda package for ujson!

@organisciak
Copy link
Collaborator

... but looks like I'll have to build an orjson conda package.

@organisciak
Copy link
Collaborator

Updated massivetexts@7eae68a with rapidjson, which seems better supported.

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

No branches or pull requests

2 participants