Skip to content

Commit

Permalink
review feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
mgeplf committed Aug 21, 2024
1 parent 2bfedf2 commit 8ab8a6c
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 12 deletions.
19 changes: 15 additions & 4 deletions src/nrnpython/utils.hpp → src/nrnpython/cast_tuple.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -3,25 +3,36 @@
#include <stdexcept>

#include <nanobind/nanobind.h>
#include <fmt/format.h>

namespace nrn {
namespace detail {

template <typename T, size_t I>
T get(const nanobind::tuple& t) {
if (I >= t.size()) {
throw std::runtime_error("Not enough arguments");
}
return nanobind::cast<T>(t[I]);
}


template <typename... Ts, typename Tuple, std::size_t... Is>
auto cast_tuple_impl(Tuple&& tuple, std::index_sequence<Is...>) {
auto count = sizeof...(Is);
if (count > tuple.size()) {
throw std::runtime_error(fmt::format(
"Not enough arguments, expected {}, but only received {}", count, tuple.size()));
}
return std::make_tuple(get<Ts, Is>(std::forward<Tuple>(tuple))...);
}
} // namespace detail

/*!
* Extract an exact number of elements from a tuple, including casting them to certain types.
*
* This uses `nanobind::cast`, so expect a `nanobind::cast_error()` if there are incompatible types.
* If the number of supplied elements in the tuple is fewer than the expected number, a std::runtime
* error is thrown.
*
* @param tuple can either be a nanobind::args or a nanobind::tuple
*/
template <typename... Ts, typename Tuple>
auto cast_tuple(Tuple&& tuple) {
return detail::cast_tuple_impl<Ts...>(std::forward<Tuple>(tuple),
Expand Down
3 changes: 2 additions & 1 deletion src/nrnpython/nrnpy_hoc.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@
#include "nrnpy_utils.h"
#include "nrnpython.h"
#include "convert_cxx_exceptions.hpp"
#include "utils.hpp"
#include "cast_tuple.hpp"

#include <unordered_map>

#include "nrnwrap_dlfcn.h"
Expand Down
23 changes: 16 additions & 7 deletions test/unit_tests/utils/cast_tuple.cpp
Original file line number Diff line number Diff line change
@@ -1,28 +1,37 @@
#include <nanobind/nanobind.h>
#include <nanobind/stl/string.h>

#include "utils.hpp"
#include "cast_tuple.hpp"

#include <catch2/catch_test_macros.hpp>

TEST_CASE("can_cast", "[Neuron][nrnpython]") {
auto args = nanobind::make_tuple(42, "test", 3.14);
TEST_CASE("can_cast_nanobind_tuple", "[Neuron][nrnpython]") {
auto tup = nanobind::make_tuple(42, "test", 3.14);
auto [i, s, d] = nrn::cast_tuple<int, std::string, double>(tup);
REQUIRE(i == 42);
REQUIRE(s == "test");
REQUIRE(d == 3.14);
}

TEST_CASE("can_cast_nanobind_args", "[Neuron][nrnpython]") {
auto tup = nanobind::make_tuple(42, "test", 3.14);
auto args = nanobind::cast<nanobind::args>(tup);
auto [i, s, d] = nrn::cast_tuple<int, std::string, double>(args);
REQUIRE(i == 42);
REQUIRE(s == "test");
REQUIRE(d == 3.14);
}

TEST_CASE("can_cast_extra_arguments", "[Neuron][nrnpython]") {
auto args = nanobind::make_tuple("test", 3.14, 42);
auto [s, d] = nrn::cast_tuple<std::string, double>(args);
auto tup = nanobind::make_tuple("test", 3.14, 42);
auto [s, d] = nrn::cast_tuple<std::string, double>(tup);
REQUIRE(s == "test");
REQUIRE(d == 3.14);
}

TEST_CASE("not_enough_arguments", "[Neuron][nrnpython]") {
auto args = nanobind::make_tuple(42, std::string("test"));
REQUIRE_THROWS(nrn::cast_tuple<int, std::string, double>(args));
auto tup = nanobind::make_tuple(42, std::string("test"));
REQUIRE_THROWS(nrn::cast_tuple<int, std::string, double>(tup));

auto empty_tuple = nanobind::tuple();
REQUIRE_THROWS(nrn::cast_tuple<int>(empty_tuple));
Expand Down

0 comments on commit 8ab8a6c

Please sign in to comment.