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.

Sign up for GitHub

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

Allow VLenUTF8 to encode a read-only source #515

Merged
merged 5 commits into from
Jul 12, 2024
Merged

Allow VLenUTF8 to encode a read-only source #515

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
0b4940f
fix?
ivirshup Mar 12, 2024
06a1424
test
ivirshup Mar 12, 2024
c822e48
format
ivirshup Mar 12, 2024
cfe1206
Merge branch 'main' into vlen-readonly
rabernat Jul 9, 2024
47f1afe
add release notes
rabernat Jul 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion docs/release.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,8 @@ Enhancements

Fix
~~~

* Fix VLenUTF8 encoding for read-only buffers.
By :user:`Isaac Virshup <ivirshup>`, :issue:`514`.
* Fix skip of entry points backport tests
By :user:`Elliott Sales de Andrade <QuLogic>`, :issue:`487`.
* Fix Upgrade to Zstd 1.5.5 due to potential corruption.
Expand Down
5 changes: 4 additions & 1 deletion numcodecs/tests/test_vlen_utf8.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,11 @@ def test_decode_errors():
codec.decode(enc, out=np.zeros(10, dtype='i4'))


def test_encode_utf8():
@pytest.mark.parametrize("writable", [True, False])
def test_encode_utf8(writable):
a = np.array(['foo', None, 'bar'], dtype=object)
if not writable:
a.setflags(write=False)
codec = VLenUTF8()
enc = codec.encode(a)
dec = codec.decode(enc)
Expand Down
3 changes: 2 additions & 1 deletion numcodecs/vlen.pyx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

import cython
cimport cython
from numpy cimport ndarray
import numpy as np
from .abc import Codec
from .compat_ext cimport Buffer
Expand Down Expand Up @@ -74,7 +75,7 @@ class VLenUTF8(Codec):
def encode(self, buf):
cdef:
Py_ssize_t i, l, n_items, data_length, total_length
object[:] input_values
ndarray[object, ndim=1] input_values
Copy link
Member

Choose a reason for hiding this comment

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

Isaac, could you please share a bit more about why this is needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't remember that much of the specifics, but it has to do with which types Cython is able to work with a read-only buffer. I remember running into this a bit with pandas too when I tried to make the index arrays actually read-only.

Is that what you were wondering about?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

More info here: #514 (comment)

Copy link
Member

Choose a reason for hiding this comment

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

Yes thank you! 🙏

object[:] encoded_values
int[:] encoded_lengths
char* encv
Expand Down
3 changes: 2 additions & 1 deletion pyproject.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@ requires = [
"setuptools>=64",
"setuptools-scm[toml]>=6.2",
"Cython",
"py-cpuinfo"
"py-cpuinfo",
"numpy",
]
build-backend = "setuptools.build_meta"

Expand Down
3 changes: 2 additions & 1 deletion setup.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -196,12 +196,13 @@ def lz4_extension():

def vlen_extension():
info('setting up vlen extension')
import numpy

extra_compile_args = base_compile_args.copy()
define_macros = []

# setup sources
include_dirs = ['numcodecs']
include_dirs = ['numcodecs', numpy.get_include()]
# define_macros += [('CYTHON_TRACE', '1')]

sources = ['numcodecs/vlen.pyx']
Expand Down