Skip to content

Commit b19e1cd

Browse files
mprekajskicopybara-github
authored andcommitted
Remove DataSlice._add_method, so that it is not accessible in public API.
PiperOrigin-RevId: 718389025 Change-Id: I74dd3e0e799155802a0275586ba60cc9389c8bd3
1 parent a45bf29 commit b19e1cd

File tree

8 files changed

+192
-175
lines changed

8 files changed

+192
-175
lines changed

py/koladata/expr/view_test.py

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -627,9 +627,7 @@ def test_data_slice_attrs_are_in_view(self):
627627
'internal_as_dense_array',
628628
'internal_as_py',
629629
'internal_is_any_schema',
630-
'internal_is_compliant_attr_name',
631630
'internal_is_itemid_schema',
632-
'internal_register_reserved_class_method_name',
633631
'is_mutable',
634632
'qtype',
635633
'set_attr',

py/koladata/types/data_item.py

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,6 @@
1919
from arolla import arolla
2020
from koladata.expr import py_expr_eval_py_ext as _py_expr_eval_py_ext
2121
from koladata.types import data_item_py_ext as _data_item_py_ext
22-
# Used to initialize DataSlice, so it is available when defining subclasses of
23-
# DataItem.
2422
from koladata.types import data_slice
2523

2624
_eval_op = _py_expr_eval_py_ext.eval_op
@@ -29,15 +27,15 @@
2927

3028

3129
### Implementation of the DataItem's additional functionality.
32-
@DataItem._add_method('__hash__') # pylint: disable=protected-access
30+
@data_slice.add_method(DataItem, '__hash__') # pylint: disable=protected-access
3331
def _hash(self) -> int:
3432
return hash(self.fingerprint)
3533

3634

3735
# Ideally we'd do this only for functors, but we don't have a fast way
3836
# to check if a DataItem is a functor now. Note that SchemaItem overrides
3937
# this behavior.
40-
@DataItem._add_method('__call__') # pylint: disable=protected-access
38+
@data_slice.add_method(DataItem, '__call__') # pylint: disable=protected-access
4139
def _call(
4240
self, *args: Any, return_type_as: Any = data_slice.DataSlice, **kwargs: Any
4341
) -> data_slice.DataSlice:
@@ -68,7 +66,7 @@ def register_bind_method_implementation(
6866

6967
# Ideally we'd do this only for functors, but we don't have a fast way
7068
# to check if a DataItem is a functor now.
71-
@DataItem._add_method('bind') # pylint: disable=protected-access
69+
@data_slice.add_method(DataItem, 'bind') # pylint: disable=protected-access
7270
def bind(
7371
self,
7472
*,

py/koladata/types/data_item_test.py

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -124,21 +124,6 @@ def test_bag(self):
124124
x = x.with_bag(db)
125125
self.assertIsNotNone(x.get_bag())
126126

127-
def test_add_method(self):
128-
# We are adding a method to DataSlice and should be visible in DataItem
129-
# after the fact.
130-
self.assertFalse(hasattr(data_item.DataItem, 'foo'))
131-
132-
@data_slice.DataSlice._add_method('foo')
133-
def foo(self):
134-
"""Converts DataSlice to DenseArray."""
135-
return self.internal_as_dense_array()
136-
137-
self.assertTrue(hasattr(data_item.DataItem, 'foo'))
138-
139-
x = ds('abc')
140-
arolla.testing.assert_qvalue_allequal(x.foo(), arolla.dense_array(['abc']))
141-
142127
def test_set_get_attr(self):
143128
db = data_bag.DataBag.empty()
144129
x = db.new(abc=ds(3.14))

py/koladata/types/data_slice.cc

Lines changed: 33 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -312,19 +312,6 @@ bool IsCompliantAttrName(absl::string_view attr_name) {
312312
attr_name);
313313
}
314314

315-
// classmethod
316-
absl::Nullable<PyObject*> PyDataSlice_is_compliant_attr_name(
317-
PyObject* cls, PyObject* attr_name) {
318-
arolla::python::DCheckPyGIL();
319-
Py_ssize_t size;
320-
const char* attr_name_ptr = PyUnicode_AsUTF8AndSize(attr_name, &size);
321-
if (attr_name_ptr == nullptr) {
322-
return nullptr;
323-
}
324-
auto attr_name_view = absl::string_view(attr_name_ptr, size);
325-
return PyBool_FromLong(IsCompliantAttrName(attr_name_view));
326-
}
327-
328315
absl::Nullable<PyObject*> PyDataSlice_getattro(PyObject* self,
329316
PyObject* attr_name) {
330317
arolla::python::DCheckPyGIL();
@@ -963,28 +950,6 @@ absl::Nullable<PyObject*> PyDataSlice_debug_repr(PyObject* self) {
963950
debug_repr.c_str(), static_cast<Py_ssize_t>(debug_repr.size()));
964951
}
965952

966-
// classmethod
967-
absl::Nullable<PyObject*>
968-
PyDataSlice_internal_register_reserved_class_method_name(
969-
PyTypeObject* cls, PyObject* method_name) {
970-
arolla::python::DCheckPyGIL();
971-
if (!PyUnicode_Check(method_name)) {
972-
PyErr_SetString(PyExc_TypeError, "method name must be a string");
973-
return nullptr;
974-
}
975-
Py_ssize_t size;
976-
const char* method_name_ptr = PyUnicode_AsUTF8AndSize(method_name, &size);
977-
if (method_name_ptr == nullptr) {
978-
return nullptr;
979-
}
980-
auto method_name_view = absl::string_view(method_name_ptr, size);
981-
if (size == 0 || method_name_view[0] != '_') {
982-
PyDataSlice_GetReservedAttrsWithoutLeadingUnderscore().insert(
983-
method_name_view);
984-
}
985-
Py_RETURN_NONE;
986-
}
987-
988953
PyMethodDef kPyDataSlice_methods[] = {
989954
{"get_bag", PyDataSlice_get_bag, METH_NOARGS,
990955
"get_bag()\n"
@@ -1220,29 +1185,12 @@ schema. In case of ANY (or primitives), an empty list is returned.
12201185
"Returns a format representation with a special support for non empty "
12211186
"specification.\n\nDataSlice will be replaced with base64 encoded "
12221187
"DataSlice.\nMust be used with kd.fstr or kde.fstr."},
1223-
{"internal_register_reserved_class_method_name",
1224-
(PyCFunction)PyDataSlice_internal_register_reserved_class_method_name,
1225-
METH_CLASS | METH_O,
1226-
"internal_register_reserved_class_method_name(method_name, /)\n"
1227-
"--\n\n"
1228-
"Registers a name to be reserved as a method of the DataSlice class.\n"
1229-
"\n"
1230-
"You must call this when adding new methods to the class in Python.\n"
1231-
"\n"
1232-
"Args:\n"
1233-
" method_name: (str)\n"},
12341188
{"_repr_with_params", (PyCFunction)PyDataSlice_repr_with_params,
12351189
METH_FASTCALL | METH_KEYWORDS,
12361190
"_repr_with_params("
12371191
"*, depth=1, unbounded_type_max_len=-1, format_html=False)\n"
12381192
"--\n\n"
12391193
"Used to generate str representation for interactive repr in Colab."},
1240-
{"internal_is_compliant_attr_name",
1241-
(PyCFunction)PyDataSlice_is_compliant_attr_name, METH_CLASS | METH_O,
1242-
"internal_is_compliant_attr_name(attr_name, /)\n"
1243-
"--\n\n"
1244-
"Returns true iff `attr_name` can be accessed through "
1245-
"`getattr(slice, attr_name)`."},
12461194
{"_debug_repr", (PyCFunction)PyDataSlice_debug_repr, METH_NOARGS,
12471195
"_debug_repr()\n"
12481196
"--\n\n"
@@ -1349,4 +1297,37 @@ PyTypeObject* PyDataSlice_Type() {
13491297
return type;
13501298
}
13511299

1300+
// data_slice_py_ext module methods
1301+
absl::Nullable<PyObject*> PyDataSliceModule_is_compliant_attr_name(
1302+
PyObject* /*module*/, PyObject* attr_name) {
1303+
arolla::python::DCheckPyGIL();
1304+
Py_ssize_t size;
1305+
const char* attr_name_ptr = PyUnicode_AsUTF8AndSize(attr_name, &size);
1306+
if (attr_name_ptr == nullptr) {
1307+
return nullptr;
1308+
}
1309+
auto attr_name_view = absl::string_view(attr_name_ptr, size);
1310+
return PyBool_FromLong(IsCompliantAttrName(attr_name_view));
1311+
}
1312+
1313+
absl::Nullable<PyObject*> PyDataSliceModule_register_reserved_class_method_name(
1314+
PyObject* /*module*/, PyObject* method_name) {
1315+
arolla::python::DCheckPyGIL();
1316+
if (!PyUnicode_Check(method_name)) {
1317+
PyErr_SetString(PyExc_TypeError, "method name must be a string");
1318+
return nullptr;
1319+
}
1320+
Py_ssize_t size;
1321+
const char* method_name_ptr = PyUnicode_AsUTF8AndSize(method_name, &size);
1322+
if (method_name_ptr == nullptr) {
1323+
return nullptr;
1324+
}
1325+
auto method_name_view = absl::string_view(method_name_ptr, size);
1326+
if (size == 0 || method_name_view[0] != '_') {
1327+
PyDataSlice_GetReservedAttrsWithoutLeadingUnderscore().insert(
1328+
method_name_view);
1329+
}
1330+
Py_RETURN_NONE;
1331+
}
1332+
13521333
} // namespace koladata::python

py/koladata/types/data_slice.h

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,25 @@
1717

1818
#include <Python.h>
1919

20+
#include "absl/base/nullability.h"
21+
2022
namespace koladata::python {
2123

2224
// Returns a PyType of Python DataSlice.
23-
PyTypeObject* PyDataSlice_Type();
25+
absl::Nullable<PyTypeObject*> PyDataSlice_Type();
26+
27+
// Returns true iff `attr_name` can be accessed through:
28+
// getattr(slice, attr_name)
29+
absl::Nullable<PyObject*> PyDataSliceModule_is_compliant_attr_name(
30+
PyObject* /*module*/, PyObject* attr_name);
31+
32+
// Registers a name in method_name to be reserved as a method of the DataSlice
33+
// class or it subclasses. For these registered method names, getattr will
34+
// invoke a PyObject_GenericGetAttr.
35+
//
36+
// This must be called when adding new methods to the class in Python.
37+
absl::Nullable<PyObject*> PyDataSliceModule_register_reserved_class_method_name(
38+
PyObject* /*module*/, PyObject* method_name);
2439

2540
} // namespace koladata::python
2641

0 commit comments

Comments
 (0)