Reference python objects directly in perspective tables #975
Conversation
|
@sc1f can you take a look and let me know your thoughts |
timkpaine
force-pushed
the
custom_repr_python
branch
from
2dae090 to
17f7670
Compare
|
@texodus ready for you to review |
cleanup vestigial code, copypasta checkpoint work handle ref count in process columns make accessor transient, don't try to incref/decref in column scope add test for update sequence, remove debug prints from stuff i understand better now implement remove update docs
texodus
force-pushed
the
custom_repr_python
branch
from
8a785ff to
1b1e8e1
Compare
There was a problem hiding this comment.
Thanks for the PR! Great test and documentation support as well, much appreciated.
This is a really exciting new feature! There were a few conflicts here with @sc1f computed columns work which I resolved through rebase. I think we should follow this up with a more rigorous examination of our perceived use case for this - there is quite alot of potential here, but Perspective's multi-runtime nature insists some "obvious" applications that are anything-but-obvious to implement.
Specifically, how do we extend our limited knowledge of an object's lifetime and identity to a more general purpose embedding, e.g. in Jupyterlab?
| } break; | ||
| case DTYPE_BOOL: | ||
| case DTYPE_UINT8: | ||
| case DTYPE_INT8: { | ||
| std::uint8_t v = 0; | ||
| set_nth<std::uint8_t>(idx, v, status); | ||
| set_nth<std::uint8_t>(idx, 0, status); |
There was a problem hiding this comment.
Any particular reason these need to be zeroed? Just curious.
| // auto col = table->get_column(colname); | ||
| // for (auto idx = 0; idx < col->size(); ++idx) { | ||
| // arr[idx] = py::cast(col->get_scalar(idx)); | ||
| // } |
| @@ -191,13 +191,19 @@ def marshal(self, cidx, ridx, dtype): | |||
| if val is None: | |||
| return val | |||
|
|
|||
| # if item implements custom repr, use it | |||
| if hasattr(val, "_psp_repr_"): | |||
| val = val._psp_repr_() | |||
There was a problem hiding this comment.
I am not a Python expert - is this canonical? __perspective_repr__?
Adds support for storing python objects in perspective tables by consolidating
DTYPE_OBJECTandDTYPE_PTRinto justDTYPE_OBJECT.Python objects are stored by pointer (e.g.
PyObject*) asuint64_tthen materialized from the underlying scalar, e.g. ifscalar.m_type == DTYPE_OBJECTwe cast theuint64_tback to aPyObject *Also added is support for value extraction via the
_psp_dtype_and_psp_repr_magic methods, the presence of which indicates how perspective should treat the returned value, along with the ability to return arbitrary values (__repr__is limited to strings, so if you want to pull an integer out of your object you can define_psp_dtype_to returnintand have_psp_repr_return anint).Along these lines, type inference is slightly modified to support promotions, e.g. if you return a custom object that supports
__int__or__float__, we'll look for those methods if you tell perspective to store as an integer or floating type.Example behaviors in the test file
Some vestigial code was removed as well, and some hardcoded
py::objectwere converted tot_valfor consistency.Todos:
id(val)won't increment the reference counter, so it will be possible for a scalar to store a pointer that has been cleaned up. When we store aDTYPE_OBJECT, we'll need to make sure toINCREFthe object._psp_dtype_and_psp_repr_