Skip to content

Issue #16: accept arbitrary buffer-compatible objects #38

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

Merged

Conversation

pitrou
Copy link
Contributor

@pitrou pitrou commented Jun 9, 2017

This removes hand-written checks for bytes and bytearray, and instead uses the buffer API which allows for arbitrary buffer-compatible types. This is similar to what standard library modules like zlib already do.

Incidentally, this also makes it possible to pass in a Numpy array without copying it to a bytes object first (see #35).

Copy link
Member

@jonathanunderwood jonathanunderwood left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, many thanks for this! One small nitpick, and one request: could you update the docstring accordingly for the source parameter to indicate that it will now accept any object supporting the buffer interface.

PyErr_Format(PyExc_OverflowError, "input too large for C 'int'");
return NULL;
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pedantic: please be consistent with error message capitalization - "Input.."

@pitrou
Copy link
Contributor Author

pitrou commented Jun 9, 2017

You're welcome. I think I've now made the changes you asked for.

@jonathanunderwood
Copy link
Member

Also, would be super awesome if you could rebase it on top of the development branch.

@pitrou
Copy link
Contributor Author

pitrou commented Jun 9, 2017

Ok, I think I've rebased now (actually merged).

@pitrou pitrou changed the base branch from master to development June 9, 2017 10:43
@pitrou
Copy link
Contributor Author

pitrou commented Jun 9, 2017

Looks like format p isn't accepted in PyArg_ParseTuple in Python 3.2 (see Travis-Ci failure), is this something you already noticed?

@pitrou
Copy link
Contributor Author

pitrou commented Jun 9, 2017

(if I may suggest so, you could simply drop Python 3.2 or even 3.3 compatibility, I don't think anybody would notice :-))

@jonathanunderwood jonathanunderwood merged commit 175bfb2 into python-lz4:development Jun 9, 2017
@jonathanunderwood
Copy link
Member

Yup - had noticed, just hadn't gotten around to removing 3.2 from the travis and appveyor configs :) Thanks again for your work on this.

@pitrou pitrou deleted the accept_buffer_objects branch June 9, 2017 10:55
pitrou added a commit to pitrou/distributed that referenced this pull request Jun 9, 2017
Current versions of the lz4 binding don't support memoryview inputs,
but later versions should (see python-lz4/python-lz4#38).
mrocklin pushed a commit to dask/distributed that referenced this pull request Jun 11, 2017
* lz4: first try passing memoryview

Current versions of the lz4 binding don't support memoryview inputs,
but later versions should (see python-lz4/python-lz4#38).

* Add tests

* Add comment about blosc

* always use blosc on memoryviews if available
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.

2 participants