Skip to content

Remove implicit node instantiation in Python #1763

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.

Sign up for GitHub

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

Closed
aljazkonec1 wants to merge 1 commit into from
Closed

Remove implicit node instantiation in Python #1763

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
5 changes: 3 additions & 2 deletions bindings/python/src/pipeline/PipelineBindings.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@

// depthai/
#include <memory>
#include <stdexcept>

#include "depthai/properties/GlobalProperties.hpp"
#include "depthai/utility/RecordReplay.hpp"
Expand Down Expand Up @@ -294,10 +295,10 @@ void PipelineBindings::bind(pybind11::module& m, void* pCallstack) {
std::shared_ptr<Node> hostNode;
try {
hostNode = py::cast<std::shared_ptr<node::ThreadedHostNode>>(class_(*args, **kwargs));
} catch(...) {
} catch(std::runtime_error& e) {
delCreatingNodeFromPipelineCreate();
delImplicitPipeline();
throw;
throw std::runtime_error(std::string("Error creating node: ") + e.what());
}
delCreatingNodeFromPipelineCreate();
delImplicitPipeline();
Expand Down
6 changes: 1 addition & 5 deletions bindings/python/src/pipeline/node/ColorCameraBindings.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,11 +91,7 @@ void bind_colorcamera(pybind11::module& m, void* pCallstack) {
.def_readwrite("warpMeshStepHeight", &ColorCameraProperties::warpMeshStepHeight)
.def_readwrite("eventFilter", &ColorCameraProperties::eventFilter);
// ColorCamera node
colorCamera
.def(py::init([]() {
auto camera = getImplicitPipeline()->create<ColorCamera>();
return camera;
}))
colorCamera.def(py::init())

@coderabbitai coderabbitai Bot Apr 21, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

Nit: style consistency — prefer py::init<>().

Other bindings in this PR (VideoEncoderBindings.cpp, StereoDepthBindings.cpp) use py::init<>() with empty angle brackets. Using py::init() here is functionally equivalent in pybind11 but inconsistent with the rest of the PR.

🎨 Proposed nit 
-    colorCamera.def(py::init())
+    colorCamera.def(py::init<>())
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
colorCamera.def(py::init())
colorCamera.def(py::init<>())
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@bindings/python/src/pipeline/node/ColorCameraBindings.cpp` at line 94, The
binding for ColorCamera uses py::init() which is inconsistent with the rest of
the PR; update the constructor registration in ColorCameraBindings.cpp by
replacing the call to colorCamera.def(py::init()) with the angle-bracket form
colorCamera.def(py::init<>()) so it matches other bindings like
VideoEncoderBindings.cpp and StereoDepthBindings.cpp.

.def_readonly("inputControl", &ColorCamera::inputControl, DOC(dai, node, ColorCamera, inputControl))
.def_readonly("initialControl", &ColorCamera::initialControl, DOC(dai, node, ColorCamera, initialControl))
.def_readonly("video", &ColorCamera::video, DOC(dai, node, ColorCamera, video))
Expand Down
10 changes: 1 addition & 9 deletions bindings/python/src/pipeline/node/DetectionNetworkBindings.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,15 +126,7 @@ void bind_detectionnetwork(pybind11::module& m, void* pCallstack) {
py::arg("fps") = std::nullopt,
DOC(dai, node, DetectionNetwork, build, 4))
#endif
.def(py::init([](DETECTION_NETWORK_BUILD_ARGS, DETECTION_NETWORK_ARGS) {
auto self = getImplicitPipeline()->create<DetectionNetwork>();
self->build(input, nnArchive);
DETECTION_NETWORK_CODE(->)
return self;
}),
DETECTION_NETWORK_BUILD_PYARGS,
DETECTION_NETWORK_PYARGS)
// Copied from NN node
.def(py::init<const std::shared_ptr<Device>&>(), py::arg("device"))
.def("setBlobPath", &DetectionNetwork::setBlobPath, py::arg("path"), DOC(dai, node, DetectionNetwork, setBlobPath))
.def("setNumPoolFrames", &DetectionNetwork::setNumPoolFrames, py::arg("numFrames"), DOC(dai, node, DetectionNetwork, setNumPoolFrames))
.def("setNumInferenceThreads",
Expand Down
17 changes: 1 addition & 16 deletions bindings/python/src/pipeline/node/StereoDepthBindings.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,22 +83,7 @@ void bind_stereodepth(pybind11::module& m, void* pCallstack) {
.value("ACCURACY", StereoDepth::PresetMode::ACCURACY);

// Node
stereoDepth
.def(py::init([](Node::Output& left, Node::Output& right, StereoDepth::PresetMode presetMode) {
auto self = getImplicitPipeline()->create<StereoDepth>();
self->build(left, right, presetMode);
return self;
}),
py::arg("left"),
py::arg("right"),
py::arg("presetMode") = StereoDepth::PresetMode::DEFAULT)
.def(py::init([](bool autoCreateCameras, StereoDepth::PresetMode presetMode) {
auto self = getImplicitPipeline()->create<StereoDepth>();
self->build(autoCreateCameras, presetMode);
return self;
}),
py::arg("autoCreateCameras"),
py::arg("presetMode") = StereoDepth::PresetMode::DEFAULT)
stereoDepth.def(py::init<>())
.def("build",
static_cast<std::shared_ptr<StereoDepth> (StereoDepth::*)(Node::Output&, Node::Output&, StereoDepth::PresetMode)>(&StereoDepth::build),
py::arg("left"),
Expand Down
9 changes: 1 addition & 8 deletions bindings/python/src/pipeline/node/VideoEncoderBindings.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,14 +74,7 @@ void bind_videoencoder(pybind11::module& m, void* pCallstack) {
},
VIDEO_ENCODER_BUILD_PYARGS,
VIDEO_ENCODER_PYARGS)
.def(py::init([](VIDEO_ENCODER_BUILD_ARGS, VIDEO_ENCODER_ARGS) {
auto self = getImplicitPipeline()->create<VideoEncoder>();
self->build(input);
VIDEO_ENCODER_CODE(->)
return self;
}),
VIDEO_ENCODER_BUILD_PYARGS,
VIDEO_ENCODER_PYARGS)
.def(py::init<>())

@coderabbitai coderabbitai Bot Apr 21, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Confirm VideoEncoder has an accessible default constructor.
ast-grep --pattern $'class VideoEncoder $_ {
  $$$
  VideoEncoder();
  $$$
}'
rg -nP -C2 '\bVideoEncoder\s*\(\s*\)\s*(;|\{|=)' include/ src/

Repository: luxonis/depthai-core

Length of output: 46

🏁 Script executed:

# Find VideoEncoder class definition
fd -type f -name "*.hpp" -o -name "*.h" | xargs rg -l "class VideoEncoder"

# Locate and read VideoEncoderBindings.cpp context
fd -type f -name "VideoEncoderBindings.cpp"

Repository: luxonis/depthai-core

Length of output: 469

🏁 Script executed:

# Search more broadly for VideoEncoder in header files
rg "class VideoEncoder" -A 10 --max-count=3

Repository: luxonis/depthai-core

Length of output: 1056

🏁 Script executed:

# Check the binding file to understand full context
wc -l bindings/python/src/pipeline/node/VideoEncoderBindings.cpp

Repository: luxonis/depthai-core

Length of output: 126

🏁 Script executed:

# Read the full VideoEncoderBindings.cpp file to see context
cat -n bindings/python/src/pipeline/node/VideoEncoderBindings.cpp

Repository: luxonis/depthai-core

Length of output: 8973

🏁 Script executed:

# Read the VideoEncoder class definition to understand constructors
cat -n include/depthai/pipeline/node/VideoEncoder.hpp

Repository: luxonis/depthai-core

Length of output: 5058

🏁 Script executed:

# Check the base class DeviceNodeCRTP to understand inherited constructors
rg "class DeviceNodeCRTP" -A 30 include/

Repository: luxonis/depthai-core

Length of output: 2542

🏁 Script executed:

# Check the actual constructor definitions in DeviceNodeCRTP
rg "DeviceNodeCRTP\s*\(" include/depthai/pipeline/DeviceNode.hpp -A 3 -B 3

Repository: luxonis/depthai-core

Length of output: 1237

🏁 Script executed:

# Also check what ADD_NODE macro does
rg "define ADD_NODE" bindings/python/src/ -A 5

Repository: luxonis/depthai-core

Length of output: 1091

🏁 Script executed:

# Check if there's a default constructor template in DeviceNodeCRTP
cat -n include/depthai/pipeline/DeviceNode.hpp | head -80

Repository: luxonis/depthai-core

Length of output: 2800

🏁 Script executed:

# Find the addNode template implementation
rg "template.*addNode" bindings/python/src/ -A 20

Repository: luxonis/depthai-core

Length of output: 46

🏁 Script executed:

# Look at NodeBindings.hpp to see how ADD_NODE works
cat -n bindings/python/src/pipeline/node/NodeBindings.hpp | head -100

Repository: luxonis/depthai-core

Length of output: 1125

🏁 Script executed:

# Look for the addNode function implementation
find bindings/python/src -name "*.hpp" -o -name "*.cpp" | xargs rg -l "addNode"

Repository: luxonis/depthai-core

Length of output: 108

🏁 Script executed:

# Check the Common.hpp file which defines ADD_NODE
cat -n bindings/python/src/pipeline/node/Common.hpp | head -150

Repository: luxonis/depthai-core

Length of output: 3066

Line 77 will fail to compile: py::init<>() cannot access the protected default constructor.

VideoEncoder inherits constructors from DeviceNodeCRTP via using DeviceNodeCRTP::DeviceNodeCRTP;, but all constructors in DeviceNodeCRTP are protected, not public. Pybind11's py::init<>() requires a public constructor to expose in Python. This binding will result in a C++ compilation error.

Either provide a public default constructor in VideoEncoder or use a custom factory-based initialization pattern (e.g., a lambda that wraps the pipeline creation logic).

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@bindings/python/src/pipeline/node/VideoEncoderBindings.cpp` at line 77, The
binding uses py::init<>() but VideoEncoder inherits only protected constructors
from DeviceNodeCRTP so the default constructor is not public; either add a
public default constructor to VideoEncoder (make VideoEncoder() public and call
the base protected constructors as needed) or replace py::init<>() with a
factory-based initializer in the pybind binding (wrap creation in a
lambda/factory that returns a new VideoEncoder or unique_ptr<VideoEncoder>),
referencing VideoEncoder, DeviceNodeCRTP and py::init<>() so the binding
compiles without needing a public base constructor.

.def_readonly("input", &VideoEncoder::input, DOC(dai, node, VideoEncoder, input), DOC(dai, node, VideoEncoder, input))
.def_readonly("bitstream", &VideoEncoder::bitstream, DOC(dai, node, VideoEncoder, bitstream), DOC(dai, node, VideoEncoder, bitstream))
.def_readonly("out", &VideoEncoder::out, DOC(dai, node, VideoEncoder, out), DOC(dai, node, VideoEncoder, out))
Expand Down
2 changes: 1 addition & 1 deletion include/depthai/pipeline/node/DetectionNetwork.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ namespace node {
*/
class DetectionNetwork : public DeviceNodeGroup {
public:
DetectionNetwork(const std::shared_ptr<Device>& device);
explicit DetectionNetwork(const std::shared_ptr<Device>& device);
using Model = NeuralNetwork::Model;

[[nodiscard]] static std::shared_ptr<DetectionNetwork> create(const std::shared_ptr<Device>& device) {
Expand Down