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

Implement pickling for PyStructList and FastList #285

Merged
merged 10 commits into from
Jun 18, 2024
2 changes: 1 addition & 1 deletion conda/dev-environment-unix.yml
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ dependencies:
- mamba
- mdformat>=0.7.17,<0.8
- ninja
- numpy
- numpy<2
- pandas
- pillow
- polars
Expand Down
2 changes: 1 addition & 1 deletion conda/dev-environment-win.yml
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ dependencies:
- mamba
- mdformat>=0.7.17,<0.8
- ninja
- numpy
- numpy<2
- pandas
- pillow
- polars
Expand Down
14 changes: 14 additions & 0 deletions cpp/csp/python/PyStructFastList_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -511,6 +511,19 @@ static PyObject * PyStructFastList_Reversed( PyStructFastList<StorageT> * self,
CSP_RETURN_NULL;
}

template<typename StorageT>
static PyObject * PyStructFastList_reduce( PyStructFastList<StorageT> * self, PyObject * Py_UNUSED( ignored) )
{
CSP_BEGIN_METHOD;

typename VectorWrapper<StorageT>::IndexType sz = self -> vector.size();
PyObjectPtr list = PyObjectPtr::own( toPython( self -> vector.getVector(), self -> arrayType ) );
ogarokpeter marked this conversation as resolved.
Show resolved Hide resolved
PyObject * result = Py_BuildValue( "O(O)", &PyList_Type, list.ptr() );
return result;

CSP_RETURN_NULL;
}

template<typename StorageT>
static PyMethodDef PyStructFastList_methods[] = {
{ "__getitem__", ( PyCFunction ) py_struct_fast_list_subscript<StorageT>, METH_VARARGS, NULL },
Expand All @@ -527,6 +540,7 @@ static PyMethodDef PyStructFastList_methods[] = {
{ "extend", ( PyCFunction ) PyStructFastList_Extend<StorageT>, METH_VARARGS, NULL },
{ "remove", ( PyCFunction ) PyStructFastList_Remove<StorageT>, METH_VARARGS, NULL },
{ "clear", ( PyCFunction ) PyStructFastList_Clear<StorageT>, METH_NOARGS, NULL },
{"__reduce__", ( PyCFunction ) PyStructFastList_reduce<StorageT>, METH_NOARGS, NULL },
{ NULL},
};

Expand Down
30 changes: 22 additions & 8 deletions cpp/csp/python/PyStructList_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -297,16 +297,30 @@ static PyObject * py_struct_list_inplace_repeat( PyObject * sself, Py_ssize_t n
CSP_RETURN_NULL;
}

template<typename StorageT>
static PyObject * PyStructList_reduce( PyStructList<StorageT> * self, PyObject * Py_UNUSED( ignored ) )
{
CSP_BEGIN_METHOD;

typename VectorWrapper<StorageT>::IndexType sz = self -> vector.size();
PyObjectPtr list = PyObjectPtr::own( toPython( self -> vector.getVector(), self -> arrayType ) );
PyObject * result = Py_BuildValue( "O(O)", &PyList_Type, list.ptr() );
return result;

CSP_RETURN_NULL;
}

template<typename StorageT>
static PyMethodDef PyStructList_methods[] = {
{ "append", ( PyCFunction ) PyStructList_Append<StorageT>, METH_VARARGS, NULL },
{ "insert", ( PyCFunction ) PyStructList_Insert<StorageT>, METH_VARARGS, NULL },
{ "pop", ( PyCFunction ) PyStructList_Pop<StorageT>, METH_VARARGS, NULL },
{ "reverse", ( PyCFunction ) PyStructList_Reverse<StorageT>, METH_NOARGS, NULL },
{ "sort", ( PyCFunction ) PyStructList_Sort<StorageT>, METH_VARARGS | METH_KEYWORDS, NULL },
{ "extend", ( PyCFunction ) PyStructList_Extend<StorageT>, METH_VARARGS, NULL },
{ "remove", ( PyCFunction ) PyStructList_Remove<StorageT>, METH_VARARGS, NULL },
{ "clear", ( PyCFunction ) PyStructList_Clear<StorageT>, METH_NOARGS, NULL },
{ "append", ( PyCFunction ) PyStructList_Append<StorageT>, METH_VARARGS, NULL },
{ "insert", ( PyCFunction ) PyStructList_Insert<StorageT>, METH_VARARGS, NULL },
{ "pop", ( PyCFunction ) PyStructList_Pop<StorageT>, METH_VARARGS, NULL },
{ "reverse", ( PyCFunction ) PyStructList_Reverse<StorageT>, METH_NOARGS, NULL },
{ "sort", ( PyCFunction ) PyStructList_Sort<StorageT>, METH_VARARGS | METH_KEYWORDS, NULL },
{ "extend", ( PyCFunction ) PyStructList_Extend<StorageT>, METH_VARARGS, NULL },
{ "remove", ( PyCFunction ) PyStructList_Remove<StorageT>, METH_VARARGS, NULL },
{ "clear", ( PyCFunction ) PyStructList_Clear<StorageT>, METH_NOARGS, NULL },
{"__reduce__", ( PyCFunction ) PyStructList_reduce<StorageT>, METH_NOARGS, NULL },
{ NULL},
};

Expand Down
38 changes: 38 additions & 0 deletions csp/tests/impl/test_struct.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import json
import numpy as np
import pickle
import pytz
import typing
import unittest
Expand Down Expand Up @@ -169,6 +170,14 @@ def __init__(self, x: int):
self.x = x


class SimpleStructForPickleList(csp.Struct):
a: typing.List[int]


class SimpleStructForPickleFastList(csp.Struct):
a: FastList[int]


# Common set of values for Struct list field tests
# For each type:
# items[:-2] are normal values of the given type that should be handled,
Expand Down Expand Up @@ -2745,6 +2754,35 @@ class B(csp.Struct):
p = csp.unroll(csp.const(A(a=[1, 2, 3])).a)
q = csp.unroll(csp.const(B(a=[1, 2, 3])).a)

def test_list_field_pickle(self):
"""Was a BUG when the struct with list field was not recognizing changes made to this field in python"""
# Not using pystruct_list_test_values, as pickling tests are of different semantics (picklability of struct fields matters).
v = [1, 5, 2]

s = SimpleStructForPickleList(a=[v[0], v[1], v[2]])

t = pickle.loads(pickle.dumps(s))

self.assertEqual(t.a, s.a)
self.assertEqual(type(t.a), type(s.a))

b = pickle.loads(pickle.dumps(s.a))

self.assertEqual(b, s.a)
self.assertEqual(type(b), list)

s = SimpleStructForPickleFastList(a=[v[0], v[1], v[2]])

t = pickle.loads(pickle.dumps(s))

self.assertEqual(t.a, s.a)
self.assertEqual(type(t.a), type(s.a))

b = pickle.loads(pickle.dumps(s.a))

self.assertEqual(b, s.a)
self.assertEqual(type(b), list)


if __name__ == "__main__":
unittest.main()
6 changes: 4 additions & 2 deletions docs/wiki/dev-guides/Build-CSP-from-Source.md
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ test run the tests

## Prerequisites

CSP has a few system-level dependencies which you can install from your machine package manager. Other package managers like `conda`, `nix`, etc, should also work fine. Currently, CSP relies on the `GNU` compiler toolchain only.
CSP has a few system-level dependencies which you can install from your machine package manager. Other package managers like `conda`, `nix`, etc, should also work fine.

## Building with Conda on Linux

Expand Down Expand Up @@ -196,13 +196,15 @@ By default, we pull and build dependencies with [vcpkg](https://vcpkg.io/en/). W

## Lint and Autoformat

CSP has listing and auto formatting.
CSP has linting and auto formatting.

| Language | Linter | Autoformatter | Description |
| :------- | :------------- | :------------- | :---------- |
| C++ | `clang-format` | `clang-format` | Style |
| Python | `ruff` | `ruff` | Style |
| Python | `isort` | `isort` | Imports |
| Markdown | `mdformat` | `mdformat` | Style |
| Markdown | `codespell` | | Spelling |

**C++ Linting**

Expand Down
2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ requires-python = ">=3.8"

dependencies = [
"backports.zoneinfo; python_version<'3.9'",
"numpy",
"numpy<2",
"packaging",
"pandas",
"psutil",
Expand Down
Loading