Required fixes
Nobody has claimed this yet.
- Dominant language
- Python
- Stars
- 63
- Forks
- 3
- PR merge metrics
- No merged PRs in 30d
Description
### ~~Default values not properly declared~~
If the function takes a default value in its arguments, it needs to be specified with `py::arg`. For example of `Vector` constructor,
```
// Constructor
.def(py::init([](py::array_t vec, bool distributed, bool copy_data = true) {
py::buffer_info buf_info = vec.request();
int dim = buf_info.shape[0];
double* data = static_cast(buf_info.ptr);
return new Vector(data, dim, distributed, copy_data);
}), py::arg("vec"), py::arg("distributed"), py::arg("copy_data") = true) // default value needs to be defined here.
```
Note the last line.
### ~~`Vector::getData` not returning an array object~~
In c++, `getData` function returns the memory address of the data array. In python, this should return the reference of the data array. Currently, `Vector::getData` does not do any special care, returning a `float` value as a result.
```
.def("getData", &Vector::getData)
```
### `Vector::get_data` naming
Compared to the function above, this function can cause a confusion. Based on its action, it needs to be renamed as `copy_data` or something.
### ~~(Enhancement) `Vector` and `Matrix` as a buffer protocol~~
`Vector` and `Matrix` should support buffer view, to handler large-size arrays without copying them in memory. Useful references are:
- [python buffer protocol](https://docs.python.org/3/c-api/buffer.html)
- [pybind11 numpy instruction](https://pybind11.readthedocs.io/en/stable/advanced/pycpp/numpy.html)
Based on [pybind11 numpy instruction](https://pybind11.readthedocs.io/en/stable/advanced/pycpp/numpy.html), an example of `Matrix` definition should be:
```
py::class_(m, "Matrix", py::buffer_protocol())
.def_buffer([](Matrix &self) -> py::buffer_info {
return py::buffer_info(
self.getData(), /* Pointer to buffer */
sizeof(float), /* Size of one scalar */
py::format_descriptor::format(), /* Python struct-style format descriptor */
2, /* Number of dimensions */
{ self.numRows(), self.numColumns() }, /* Buffer dimensions */
{ sizeof(float) * self.numColumns(),
sizeof(float) } /* Strides (in bytes) for each index */
);
})
```
Similar definition is possible for `Vector` as well.
### (Enhancement) Supporting slice view of `Vector` and `Matrix`
`Vector` and `Matrix` must support slice view of its data.
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start by locating the Vector and Matrix pybind11 bindings and compare their current APIs with the issue's naming and slice-view requirements. Use the linked Python buffer protocol and pybind11 NumPy references as the design starting point. Done means the remaining naming concern and slice-view support are resolved for both types and verified by relevant tests.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp, python
- Domain
- api, backend
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 30/100