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

Releaseable point cloud #24

Merged
merged 1 commit into from
Nov 12, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .flake8-packaged-files
Original file line number Diff line number Diff line change
Expand Up @@ -11,4 +11,5 @@ ignore =
### Temporarily disabled docstring errors:
D105, # missing docstring in magic method
I401, # Missing exception(s) in Raises section
D402, # First line should not be the function’s “signature”
show-source = True
1 change: 1 addition & 0 deletions modules/zivid/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
)
import zivid.environment
import zivid.firmware
from zivid.point_cloud import PointCloud
from zivid.frame import Frame # noqa: F401 'zivid.frame.Frame' imported but unused
from zivid.frame_info import (
FrameInfo, # noqa: F401 'zivid.frame_info.FrameInfo' imported but unused
Expand Down
6 changes: 3 additions & 3 deletions modules/zivid/frame.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
"""Contains the Frame class."""
from pathlib import Path
import numpy
import _zivid

import zivid._settings_converter as _settings_converter
import zivid._camera_state_converter as _camera_state_converter
import zivid._frame_info_converter as _frame_info_converter
from zivid.point_cloud import PointCloud


class Frame: # pylint: disable=too-few-public-methods
Expand Down Expand Up @@ -36,10 +36,10 @@ def get_point_cloud(self):
"""Copy the point cloud to the CPU and returns it.

Returns:
A numpy array
a point cloud instance

"""
return numpy.array(self.__impl.get_point_cloud())
return PointCloud(self.__impl.get_point_cloud())

def save(self, file_path):
"""Save the frame to file. The file type is determined from the file extension.
Expand Down
66 changes: 66 additions & 0 deletions modules/zivid/point_cloud.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
"""Contains the PointCloud class."""
import numpy

import _zivid


class PointCloud:
"""A point cloud."""

def __init__(self, internal_point_cloud):
"""Create a point cloud from an internal point cloud.

Args:
internal_point_cloud: a internal point cloud

"""
if not isinstance(internal_point_cloud, _zivid.PointCloud):
raise ValueError(
"Unsupported type for argument internal_point_cloud. Got {}, expected {}".format(
type(internal_point_cloud), type(_zivid.PointCloud)
)
)
self.__impl = internal_point_cloud

def to_array(self):
"""Convert point cloud to numpy array.

Returns:
a numpy array

"""
self.__impl.assert_not_released()
return numpy.array(self.__impl)

@property
def height(self):
"""Return height (number of rows) of point cloud.

Returns:
a positive integer

"""
return self.__impl.height()

@property
def width(self):
"""Return width (number of columns) of point cloud.

Returns:
a positive integer

"""
return self.__impl.width()

def release(self):
"""Release the underlying resources."""
self.__impl.release()

def __enter__(self):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should have a mixin or base class or something for these three functions. They are duplicated many places now.

You are just adding to the existing solution, so no need to fix in this PR. But if you agree please add an issue for it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could probably be done through a decorator:

@utils.context_manager
class SomethingThatRequiresContextManager:
    ....

Will create an issue

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#27

return self

def __exit__(self, exception_type, exception_value, traceback):
self.release()

def __del__(self):
self.release()
2 changes: 1 addition & 1 deletion src/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ set(SOURCES
Environment.cpp
Firmware.cpp
HDR.cpp
PointCloud.cpp
ReleasablePointCloud.cpp
ReleasableCamera.cpp
ReleasableFrame.cpp
SingletonApplication.cpp
Expand Down
11 changes: 7 additions & 4 deletions src/PointCloud.cpp → src/ReleasablePointCloud.cpp
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#include <ZividPython/PointCloud.h>
#include <ZividPython/ReleasablePointCloud.h>

#include <pybind11/pybind11.h>

Expand All @@ -18,7 +18,7 @@ namespace
};
#pragma pack(pop)

py::buffer_info makeBufferInfo(Zivid::PointCloud &pointCloud)
py::buffer_info makeBufferInfo(ZividPython::ReleasablePointCloud &pointCloud)
{
const auto data = pointCloud.dataPtr();

Expand All @@ -45,10 +45,13 @@ namespace

namespace ZividPython
{
void wrapClass(pybind11::class_<Zivid::PointCloud> pyClass)
void wrapClass(pybind11::class_<ReleasablePointCloud> pyClass)
{
PYBIND11_NUMPY_DTYPE(DataType, x, y, z, contrast, b, g, r, a);

pyClass.def(py::init<>()).def_buffer(makeBufferInfo);
pyClass.def(py::init<>())
.def_buffer(makeBufferInfo)
.def("width", &ReleasablePointCloud::width)
.def("height", &ReleasablePointCloud::height);
}
} // namespace ZividPython
4 changes: 2 additions & 2 deletions src/Wrapper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,9 @@
#include <ZividPython/Environment.h>
#include <ZividPython/Firmware.h>
#include <ZividPython/HDR.h>
#include <ZividPython/PointCloud.h>
#include <ZividPython/ReleasableCamera.h>
#include <ZividPython/ReleasableFrame.h>
#include <ZividPython/ReleasablePointCloud.h>
#include <ZividPython/SingletonApplication.h>
#include <ZividPython/Version.h>
#include <ZividPython/Wrapper.h>
Expand All @@ -30,7 +30,7 @@ ZIVID_PYTHON_MODULE // NOLINT
ZIVID_PYTHON_WRAP_CLASS_AS_RELEASABLE(module, Frame);
ZIVID_PYTHON_WRAP_CLASS(module, CameraRevision);

ZIVID_PYTHON_WRAP_CLASS_BUFFER(module, PointCloud);
ZIVID_PYTHON_WRAP_CLASS_BUFFER_AS_RELEASABLE(module, PointCloud);

ZIVID_PYTHON_WRAP_NAMESPACE_AS_SUBMODULE(module, Environment);
ZIVID_PYTHON_WRAP_NAMESPACE_AS_SUBMODULE(module, Firmware);
Expand Down
9 changes: 0 additions & 9 deletions src/include/ZividPython/PointCloud.h

This file was deleted.

7 changes: 7 additions & 0 deletions src/include/ZividPython/Releasable.h
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,13 @@ namespace ZividPython
m_impl.reset();
}

// This function is required to verify that the buffer has not already
// released in certain situations. Ideally it should not exist.
void assertNotReleased()
{
std::ignore = impl();
}

private:
std::optional<T> m_impl{ std::make_optional<T>() };
};
Expand Down
3 changes: 2 additions & 1 deletion src/include/ZividPython/ReleasableFrame.h
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

#include <Zivid/Frame.h>
#include <ZividPython/Releasable.h>
#include <ZividPython/ReleasablePointCloud.h>
#include <ZividPython/Wrappers.h>

namespace ZividPython
Expand All @@ -13,7 +14,7 @@ namespace ZividPython

ZIVID_PYTHON_FORWARD_1_ARGS(save, const std::string &, fileName)
ZIVID_PYTHON_FORWARD_1_ARGS(load, const std::string &, fileName)
ZIVID_PYTHON_FORWARD_0_ARGS(getPointCloud)
ZIVID_PYTHON_FORWARD_0_ARGS_WRAP_RETURN(ReleasablePointCloud, getPointCloud)
ZIVID_PYTHON_FORWARD_0_ARGS(settings)
ZIVID_PYTHON_FORWARD_0_ARGS(state)
ZIVID_PYTHON_FORWARD_0_ARGS(info)
Expand Down
20 changes: 20 additions & 0 deletions src/include/ZividPython/ReleasablePointCloud.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
#pragma once

#include <Zivid/PointCloud.h>
#include <ZividPython/Releasable.h>
#include <ZividPython/Wrappers.h>

namespace ZividPython
{
class ReleasablePointCloud : public Releasable<Zivid::PointCloud>
{
public:
using Releasable<Zivid::PointCloud>::Releasable;

ZIVID_PYTHON_FORWARD_0_ARGS(width)
ZIVID_PYTHON_FORWARD_0_ARGS(height)
ZIVID_PYTHON_FORWARD_0_ARGS(dataPtr)
};

void wrapClass(pybind11::class_<ReleasablePointCloud> pyClass);
} // namespace ZividPython
8 changes: 4 additions & 4 deletions src/include/ZividPython/Wrappers.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ namespace ZividPython

if constexpr(WrapType::releasable == wrapType)
{
pyClass.def("release", &Source::release);
pyClass.def("release", &Source::release).def("assert_not_released", &Source::assertNotReleased);
}
else if constexpr(WrapType::singleton == wrapType)
{
Expand Down Expand Up @@ -63,10 +63,10 @@ namespace ZividPython
ZividPython::wrapClass<ZividPython::Singleton##name, ZividPython::WrapType::singleton>( \
dest, static_cast<void (*)(pybind11::class_<ZividPython::Singleton##name>)>(ZividPython::wrapClass), #name);

#define ZIVID_PYTHON_WRAP_CLASS_BUFFER(dest, name) \
ZividPython::wrapClass<Zivid::name, ZividPython::WrapType::normal>( \
#define ZIVID_PYTHON_WRAP_CLASS_BUFFER_AS_RELEASABLE(dest, name) \
ZividPython::wrapClass<ZividPython::Releasable##name, ZividPython::WrapType::releasable>( \
dest, \
static_cast<void (*)(pybind11::class_<Zivid::name>)>(ZividPython::wrapClass), \
static_cast<void (*)(pybind11::class_<ZividPython::Releasable##name>)>(ZividPython::wrapClass), \
#name, \
pybind11::buffer_protocol())

Expand Down
6 changes: 6 additions & 0 deletions test/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,12 @@ def frame_fixture(application, sample_data_file): # pylint: disable=unused-argu
yield frame


@pytest.fixture(name="point_cloud")
def point_cloud_fixture(frame):
with frame.get_point_cloud() as point_cloud:
yield point_cloud


@pytest.fixture(name="random_settings")
def random_settings_fixture():
import datetime
Expand Down
4 changes: 2 additions & 2 deletions test/test_frame.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,10 @@ def test_illegal_init(application): # pylint: disable=unused-argument


def test_get_point_cloud(frame):
import numpy
import zivid

point_cloud = frame.get_point_cloud()
assert isinstance(point_cloud, numpy.ndarray)
assert isinstance(point_cloud, zivid.PointCloud)


def test_release(frame):
Expand Down
68 changes: 61 additions & 7 deletions test/test_point_cloud.py
Original file line number Diff line number Diff line change
@@ -1,17 +1,71 @@
def test_point_cloud_to_array(frame):
def test_point_cloud_to_array(point_cloud):
import numpy as np

point_cloud = frame.get_point_cloud()
np_array = np.array(point_cloud)
np_array = point_cloud.to_array()
assert np_array is not None
assert isinstance(np_array, np.ndarray)


def test_to_rgb_image(frame):
def test_to_rgb_image(point_cloud):
import numpy as np

point_cloud = frame.get_point_cloud()
image = point_cloud[["r", "g", "b"]]
image = np.asarray([point_cloud["r"], point_cloud["g"], point_cloud["b"]])
np_array = point_cloud.to_array()
image = np_array[["r", "g", "b"]]
image = np.asarray([np_array["r"], np_array["g"], np_array["b"]])
image = np.moveaxis(image, [0, 1, 2], [2, 0, 1])
image = image.astype(np.uint8)


def test_height(point_cloud):
height = point_cloud.height

assert height is not None
assert isinstance(height, int)


def test_width(point_cloud):
width = point_cloud.width

assert width is not None
assert isinstance(width, int)


def test_height_context_manager(frame):
import pytest

with frame.get_point_cloud() as point_cloud:
point_cloud.height # pylint: disable=pointless-statement
with pytest.raises(RuntimeError):
point_cloud.height # pylint: disable=pointless-statement


def test_width_context_manager(frame):
import pytest

with frame.get_point_cloud() as point_cloud:
point_cloud.width # pylint: disable=pointless-statement
with pytest.raises(RuntimeError):
point_cloud.width # pylint: disable=pointless-statement


def test_to_array_context_manager(frame):
import pytest

with frame.get_point_cloud() as point_cloud:
point_cloud.to_array()
with pytest.raises(RuntimeError):
point_cloud.to_array()


def test_illegal_init(application): # pylint: disable=unused-argument
import pytest
import zivid

with pytest.raises(TypeError):
zivid.PointCloud() # pylint: disable=no-value-for-parameter

with pytest.raises(ValueError):
zivid.PointCloud("Should fail.")

with pytest.raises(ValueError):
zivid.PointCloud(123)