Implement get() to get a value#16
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds a public get(id) API for tables and columns, backed by a new internal Groonga value-fetch implementation and Python type conversion, and extends tests to validate the behavior.
Changes:
- Add
get()toTableandColumnto retrieve previouslyset()values. - Implement
_get_value()and bulk-to-Python conversion inObjectusinggrn_obj_get_value(). - Add tests covering
get()for table values, scalar columns, and “no data” defaults.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| testing/test_table.py | Adds an assertion that Table.set() values can be retrieved via Table.get(). |
| testing/test_column.py | Adds Column.get() assertions for multiple scalar types and default/no-data cases. |
| grnpy/table.pyx | Exposes Table.get(id) that delegates to _get_value. |
| grnpy/column.pyx | Exposes Column.get(id) that delegates to _get_value. |
| grnpy/object.pyx | Implements _get_value() via grn_obj_get_value() and converts Groonga bulk values to Python types. |
| grnpy/object.pxd | Exposes _get_value() and _bulk_to_python() to Cython. |
| grnpy/grnpy_obj.h | Adds C helper declarations for initializing/inspecting bulk objects. |
| grnpy/grnpy_obj.c | Implements C helpers for bulk init/domain/head/size. |
| grnpy/grn_type.pxd | Adds Groonga domain/type constants used for bulk conversion. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
efc90b5 to
b35b815
Compare
| import grnpy.initializer | ||
|
|
||
| cdef extern from "stdbool.h": | ||
| ctypedef bint c_bool "bool" |
There was a problem hiding this comment.
If we simply use bool, it will be treated as Python's bool and cause an error.
We're using it by giving C's bool an alias.
There was a problem hiding this comment.
https://cython.readthedocs.io/en/latest/src/userguide/faq.html#how-do-i-declare-an-object-of-type-bool
In the case of bool in C++, we can use cimport.
There was a problem hiding this comment.
Why do we need to use ctypedef not cimport here?
There was a problem hiding this comment.
Since there is no .pxd file like there is for stdint, using ctypedef instead.
$ ls /opt/venv/lib/python3.10/site-packages/Cython/Includes/libc/stdint.pxd
/opt/venv/lib/python3.10/site-packages/Cython/Includes/libc/stdint.pxd
$ ls /opt/venv/lib/python3.10/site-packages/Cython/Includes/libc/stdbool.pxd
ls: cannot access '/opt/venv/lib/python3.10/site-packages/Cython/Includes/libc/stdbool.pxd': No such file or directory
No description provided.