From bdc91f4a3ff78f7a23c454ab94d867d9e6e87b1f Mon Sep 17 00:00:00 2001
From: Alexey Shlyonskikh <dedantysag2@yandex.ru>
Date: Wed, 14 Jun 2023 23:56:25 +0300
Subject: [PATCH] Reset algorithm configuration when used with kwargs

Also updated comments to explicitly state the supported scenarios
---
 python_bindings/py_algorithm.cpp | 13 ++++++++++---
 python_bindings/py_algorithm.h   | 27 +++++++++++++++++++++------
 2 files changed, 31 insertions(+), 9 deletions(-)

diff --git a/python_bindings/py_algorithm.cpp b/python_bindings/py_algorithm.cpp
index 787b6030e0..26601d89e0 100644
--- a/python_bindings/py_algorithm.cpp
+++ b/python_bindings/py_algorithm.cpp
@@ -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) {
@@ -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;
@@ -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();
 }
@@ -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
diff --git a/python_bindings/py_algorithm.h b/python_bindings/py_algorithm.h
index 34b519d4f7..05c1f3347b 100644
--- a/python_bindings/py_algorithm.h
+++ b/python_bindings/py_algorithm.h
@@ -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 = {});
 
@@ -25,6 +42,7 @@ 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;
@@ -32,17 +50,14 @@ class PyAlgorithmBase {
     [[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);