Skip to content
This repository has been archived by the owner on Dec 11, 2023. It is now read-only.

EAFP when reading schunk and use ujson if available #346

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

EAFP when reading schunk and use ujson if available #346

wants to merge 1 commit into from

Conversation

bordingj
Copy link

@bordingj bordingj commented May 7, 2017

@bordingj bordingj changed the title EAFP when reading schunk and use ujson if avaible EAFP when reading schunk and use ujson if available May 7, 2017
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.3%) to 65.114% when pulling ba25bb3 on bordingj:EAFP into 8df64b4 on Blosc:master.

@FrancescAlted
Copy link
Member

Looks good to me. Any performance hints on how much this PR can accelerate things? Could you add some estimates to the RELEASE_NOTES files?

@bordingj
Copy link
Author

In my benchmarks read-speed is ~2% faster

@FrancescAlted
Copy link
Member

Ok, so 2% is not really significant, although your modifications are not too intrusive, so I am still open to accept this PR. Thanks!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@bordingj @coveralls @FrancescAlted