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

Add DES python bindings #508

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

VanyaVolgushev
Copy link
Contributor

No description provided.

@VanyaVolgushev VanyaVolgushev force-pushed the add-des-bindings-release branch from f909e81 to 951faac Compare January 3, 2025 22:50
@VanyaVolgushev VanyaVolgushev marked this pull request as ready for review January 3, 2025 23:05
@VanyaVolgushev VanyaVolgushev force-pushed the add-des-bindings-release branch from 951faac to 3395d12 Compare January 7, 2025 17:52
@chernishev chernishev requested a review from BUYT-1 January 7, 2025 19:24
@VanyaVolgushev VanyaVolgushev force-pushed the add-des-bindings-release branch from 21d8424 to 637813d Compare January 8, 2025 20:24
@VanyaVolgushev VanyaVolgushev force-pushed the add-des-bindings-release branch from 637813d to b93e464 Compare January 10, 2025 13:09
Comment on lines +112 to +116
for i in range(0, len(hound_row_strings)):
if i-1 in red_line_indices:
print(CLR['red_bg'], hound_row_strings[i], CLR['default_bg'])
else:
print(hound_row_strings[i])
Copy link
Collaborator

@BUYT-1 BUYT-1 Jan 10, 2025

Choose a reason for hiding this comment

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

Suggested change
for i in range(0, len(hound_row_strings)):
if i-1 in red_line_indices:
print(CLR['red_bg'], hound_row_strings[i], CLR['default_bg'])
else:
print(hound_row_strings[i])
for i, hound_row_string in enumerate(hound_row_strings, start=-1):
if i in red_line_indices:
print(CLR['red_bg'], hound_row_string, CLR['default_bg'])
else:
print(hound_row_string)

Copy link
Collaborator

Choose a reason for hiding this comment

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

That -1 doesn't look right, are you sure that's what's supposed to be here?

Comment on lines +55 to +56
.def_property_readonly("ante", &NAR::GetAnte, py::return_value_policy::move)
.def_property_readonly("cons", &NAR::GetCons, py::return_value_policy::move);
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think the move will do anything, it will try to std::move a const.

Suggested change
.def_property_readonly("ante", &NAR::GetAnte, py::return_value_policy::move)
.def_property_readonly("cons", &NAR::GetCons, py::return_value_policy::move);
.def_property_readonly("ante", &NAR::GetAnte)
.def_property_readonly("cons", &NAR::GetCons);

Comment on lines +58 to +63
py::class_<NARAlgorithm, Algorithm>(nar_module, "NarAlgorithm")
.def("get_nars", &NARAlgorithm::GetNARVector);

auto algos_module = nar_module.def_submodule("algorithms");
auto default_algorithm = detail::RegisterAlgorithm<DES, NARAlgorithm>(algos_module, "des");
algos_module.attr("Default") = default_algorithm;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
py::class_<NARAlgorithm, Algorithm>(nar_module, "NarAlgorithm")
.def("get_nars", &NARAlgorithm::GetNARVector);
auto algos_module = nar_module.def_submodule("algorithms");
auto default_algorithm = detail::RegisterAlgorithm<DES, NARAlgorithm>(algos_module, "des");
algos_module.attr("Default") = default_algorithm;
BindPrimitive<DES>(nar_module, &NARAlgorithm::GetNARVector, "NarAlgorithm", "get_nars", {"DES"}, pybind11::return_value_policy::copy);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants