Skip to content

Commit

Permalink
Reset algorithm configuration when used with kwargs
Browse files Browse the repository at this point in the history
Also updated comments to explicitly state the supported scenarios
  • Loading branch information
BUYT-1 committed Jun 14, 2023
1 parent d571eb2 commit bdc91f4
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 9 deletions.
13 changes: 10 additions & 3 deletions python_bindings/py_algorithm.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ namespace py = pybind11;

void PyAlgorithmBase::Configure(py::kwargs const& kwargs,
std::function<boost::any()> const& create_data_reader) {
algorithm_->ResetConfiguration();
set_option_used_on_stage_ = false;
algos::ConfigureFromFunction(*algorithm_, [this, &kwargs,
create_data_reader](std::string_view option_name) {
if (option_name == kRelation && create_data_reader) {
Expand All @@ -38,6 +40,7 @@ void PyAlgorithmBase::Configure(py::kwargs const& kwargs,
}

void PyAlgorithmBase::SetOption(std::string_view option_name, py::handle option_value) {
set_option_used_on_stage_ = true;
if (option_value.is_none()) {
algorithm_->SetOption(option_name);
return;
Expand All @@ -60,7 +63,6 @@ py::frozenset PyAlgorithmBase::GetOptionType(std::string_view option_name) const

void PyAlgorithmBase::LoadProvidedData(pybind11::kwargs const& kwargs,
std::function<boost::any()> const& create_data_reader) {
algorithm_->UnsetOption(kRelation);
Configure(kwargs, create_data_reader);
algorithm_->LoadData();
}
Expand All @@ -81,11 +83,16 @@ void PyAlgorithmBase::LoadData(py::handle dataframe, std::string name, py::kwarg

void PyAlgorithmBase::LoadData() {
algorithm_->LoadData();
set_option_used_on_stage_ = false;
}

py::int_ PyAlgorithmBase::Execute(py::kwargs const& kwargs) {
Configure(kwargs);
return algorithm_->Execute();
if (!(set_option_used_on_stage_ && kwargs.empty())) {
Configure(kwargs);
}
py::int_ time = algorithm_->Execute();
set_option_used_on_stage_ = false;
return time;
}

} // namespace python_bindings
27 changes: 21 additions & 6 deletions python_bindings/py_algorithm.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,25 @@

namespace python_bindings {

// Currently, only two scenarios are supported. Either
// (scenario 1)
// 1. create the algorithm
// 2. set options using `set_option`
// 3. call `load_data` with no arguments
// 4. set options using `set_option`
// 5. call `execute` with no arguments
// or
// (scenario 2)
// 1. create the algorithm
// 2. call `load_data` with option arguments
// 3. call `execute` with option arguments
// Using the class in any other way from Python is undefined behaviour.
// scenario 2 is intended for use from Python's interactive shell
// scenario 1 is intended for use in scripts where parameters are set one-by-one
class PyAlgorithmBase {
private:
bool set_option_used_on_stage_ = false;

void LoadProvidedData(pybind11::kwargs const& kwargs,
std::function<boost::any()> const& create_data_reader = {});

Expand All @@ -25,24 +42,22 @@ class PyAlgorithmBase {
std::function<boost::any()> const& create_data_reader = {});

public:
// (scenario 1)
void SetOption(std::string_view option_name, pybind11::handle option_value);

[[nodiscard]] std::unordered_set<std::string_view> GetNeededOptions() const;

[[nodiscard]] pybind11::frozenset GetOptionType(std::string_view option_name) const;

// For pandas dataframes
// (scenario 2)
void LoadData(pybind11::handle dataframe, std::string name, pybind11::kwargs const& kwargs);

// (scenario 2)
void LoadData(std::string_view path, char separator, bool has_header,
pybind11::kwargs const& kwargs);

// For the case when the "data" option has been set by `set_option`.
// Currently, there is no use case where we want to set other options in
// bulk with kwargs when the "data" option is set by `set_option`, so this
// method assumes that all the other data loading options have already been
// set by `set_option` as well and doesn't accept any arguments. This only
// moves the algorithm to the execution stage.
// (scenario 1)
void LoadData();

pybind11::int_ Execute(pybind11::kwargs const& kwargs);
Expand Down

0 comments on commit bdc91f4

Please sign in to comment.