[glom] Python calculations and scripts: Fix regression in use of date and time fields.

[Murray Cumming <murrayc src gnome org>]

commit 05669fb9d473201da3264fa315e06d9330d92c0b
Author: Murray Cumming <murrayc murrayc com>
Date:   Mon Apr 12 23:23:01 2010 +0200

    Python calculations and scripts: Fix regression in use of date and time fields.
    
    * glom/libglom/python_embed/py_glom_record.[h|cc]:
    Change PyGlomRecord_SetFields() to a member set_fields() method, and making
    more of the class private, finishing our conversion to boost::python.
    * glom/libglom/python_embed/py_glom_related.[h|cc]:
    Change PyGlomRelated_SetRelationships() to a set_relationships methods.
    * glom/libglom/python_embed/py_glom_relatedrecord.[h|cc]:
    Changed PyGlomRelatedRecord_SetRelationship() to a set_relationship() method.
    
    * glom/python_embed/glom_python.cc: glom_pygda_value_from_pyobject():
      Handle date, time, timestamp, geometric point values, whose code had been
      commented out since the port to boost::python.
    This fixes the test added in the previous commit, so glom calculations and
    scripts can again (it broke in unstable Glom 1.13) use date and time field
    values from the record.

 ChangeLog                                          |   19 +++++
 glom/libglom/python_embed/py_glom_record.cc        |   79 ++++----------------
 glom/libglom/python_embed/py_glom_record.h         |   24 +++---
 glom/libglom/python_embed/py_glom_related.cc       |    6 +-
 glom/libglom/python_embed/py_glom_related.h        |    8 +-
 glom/libglom/python_embed/py_glom_relatedrecord.cc |   10 +--
 glom/libglom/python_embed/py_glom_relatedrecord.h  |    6 +-
 .../libglom/python_embed/pygdavalue_conversions.cc |   45 +++++++++---
 glom/python_embed/glom_python.cc                   |    4 +-
 tests/test_python_execute_func_date.cc             |   46 +++++++++--
 10 files changed, 135 insertions(+), 112 deletions(-)
---
diff --git a/ChangeLog b/ChangeLog
index a08413e..12527a4 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,24 @@
 2010-04-12  Murray Cumming  <murrayc murrayc com>
 
+  Python calculations and scripts: Fix regression in use of date and time fields.
+  
+	* glom/libglom/python_embed/py_glom_record.[h|cc]:
+	Change PyGlomRecord_SetFields() to a member set_fields() method, and making 
+	more of the class private, finishing our conversion to boost::python.
+	* glom/libglom/python_embed/py_glom_related.[h|cc]: 
+	Change PyGlomRelated_SetRelationships() to a set_relationships methods.
+	* glom/libglom/python_embed/py_glom_relatedrecord.[h|cc]:
+	Changed PyGlomRelatedRecord_SetRelationship() to a set_relationship() method.
+	
+	* glom/python_embed/glom_python.cc: glom_pygda_value_from_pyobject():
+  Handle date, time, timestamp, geometric point values, whose code had been 
+  commented out since the port to boost::python.
+	This fixes the test added in the previous commit, so glom calculations and 
+	scripts can again (it broke in unstable Glom 1.13) use date and time field 
+	values from the record.
+
+2010-04-12  Murray Cumming  <murrayc murrayc com>
+
   Added test showing problem with date fields in python calculations.
   
 	* tests/test_python_execute_func_date.cc: Added a test of dates as input 
diff --git a/glom/libglom/python_embed/py_glom_record.cc b/glom/libglom/python_embed/py_glom_record.cc
index 9affd43..4773161 100644
--- a/glom/libglom/python_embed/py_glom_record.cc
+++ b/glom/libglom/python_embed/py_glom_record.cc
@@ -92,7 +92,7 @@ boost::python::object PyGlomRecord::get_related()
     if(extractor.check())
     {
       PyGlomRelated* related_cpp = extractor;
-      PyGlomRelated_SetRelationships(related_cpp, map_relationships);
+      related_cpp->set_relationships(map_relationships);
       related_cpp->m_record = boost::python::object(this); //TODO_NotSure
     }
   }
@@ -241,74 +241,25 @@ void PyGlomRecord::setitem(const boost::python::object& key, const boost::python
   //TODO: Do dependent calculations and lookups. Or just do them for all fields for this record when finishing the script?
 }
 
-void PyGlomRecord_SetFields(PyGlomRecord* self, const PyGlomRecord::type_map_field_values& field_values, Document* document, const Glib::ustring& table_name, const sharedptr<const Field>& key_field, const Gnome::Gda::Value& key_field_value, const Glib::RefPtr<Gnome::Gda::Connection>& opened_connection)
+void PyGlomRecord::set_fields(const PyGlomRecord::type_map_field_values& field_values, Document* document, const Glib::ustring& table_name, const sharedptr<const Field>& key_field, const Gnome::Gda::Value& key_field_value, const Glib::RefPtr<Gnome::Gda::Connection>& opened_connection)
 {
-  g_assert(self);
-
-  self->m_map_field_values = field_values;
-
-  self->m_table_name = table_name;
-  self->m_key_field = key_field;
-  self->m_key_field_value = key_field_value;
-
-  if(self->m_document == 0)
-    self->m_document = document;
-
-  self->m_connection = opened_connection;
-
-  /*
-  if(self->m_fields_dict == 0)
-    self->m_fields_dict = PyDict_New();
-
-  PyDict_Clear( self->m_fields_dict );
-
-  //TODO: Cache this in one place:
-  PyObject* module_gda = PyImport_ImportModule("gda");
-  if(!module_gda)
-    g_warning("Could not import python gda module.");
-
-  PyObject* module_gda_dict = PyModule_GetDict(module_gda);
-  PyObject* pyTypeGdaValue = PyDict_GetItemString(module_gda_dict, "Value"); //TODO: Unref this?
-  if(!pyTypeGdaValue || !PyType_Check(pyTypeGdaValue))
-    g_warning("Could not get gda.Value from gda_module.");
-
-  //Add the new pairs:
-  for(type_map_fields::const_iterator iter = fields.begin(); iter != fields.end(); ++iter)
+  m_map_field_values = field_values;
+  /* Just for debugging:
+  for(type_map_field_values::const_iterator iter = field_values.begin(); iter != field_values.end(); ++iter)
   {
-    //Add each name/value pair:
-
-    //PyObject* pyValue = _PyObject_New((PyTypeObject*)pyTypeGdaValue);
-    //if(!pyValue)
-    //  g_warning("_PyObject_New() failed.");
-
-    //PyObject_New() does not call the derived constructor. Stupid PyObject_New().
-    //PyObject* new_args = PyTuple_New(0);
-    //pyValue->ob_type->tp_init(pyValue, new_args, 0);
-    //Py_DECREF(new_args);
-
-    //PyObject_Call() instantiates a type when passed that type as the object to call. Strange Python.
-    PyObject* new_args = PyTuple_New(0);
-    PyObject* pyValue = PyObject_Call(pyTypeGdaValue, new_args, 0);
-    Py_DECREF(new_args);
-    if(!pyValue)
-    {
-      g_warning("PyObject_Call() failed to instantiate the type.");
-      Record_HandlePythonError();
-    }
-
-    //g_warning("pyValue->op_type->tp_name=%s", pyValue->ob_type->tp_name);
-
-    PyGBoxed* pygBoxed = (PyGBoxed*)(pyValue);
-    GdaValue* pGdaValue = (GdaValue*)pygBoxed->boxed;
+    const Gnome::Gda::Value value = iter->second;
+    std::cout << "DEBUG: PyGlomRecord::set_fields(): field name=" << iter->first << ", type=" << g_type_name(value.get_value_type()) << std::endl;
+  }
+  */
 
-    if(!pGdaValue)
-      g_warning("pygBoxed->boxed is NULL");
+  m_table_name = table_name;
+  m_key_field = key_field;
+  m_key_field_value = key_field_value;
 
-    gda_value_set_from_value(pGdaValue, (iter->second).gobj());
+  if(m_document == 0)
+    m_document = document;
 
-    PyDict_SetItemString(self->m_fields_dict, iter->first.c_str(), pyValue);
-  }
-  */
+  m_connection = opened_connection;
 }
 
 } //namespace Glom
diff --git a/glom/libglom/python_embed/py_glom_record.h b/glom/libglom/python_embed/py_glom_record.h
index eaec45e..2ace962 100644
--- a/glom/libglom/python_embed/py_glom_record.h
+++ b/glom/libglom/python_embed/py_glom_record.h
@@ -57,31 +57,33 @@ public:
   boost::python::object getitem(const boost::python::object& item);
   void setitem(const boost::python::object& /* key */, const boost::python::object& /* value */);
 
+  //Available, for instance, in python via record["name_first"]
+  typedef std::map<Glib::ustring, Gnome::Gda::Value> type_map_field_values;
+
+  void set_fields(const PyGlomRecord::type_map_field_values& field_values,
+    Document* document,
+    const Glib::ustring& table_name,
+    const sharedptr<const Field>& key_field,
+    const Gnome::Gda::Value& key_field_value,
+    const Glib::RefPtr<Gnome::Gda::Connection>& opened_connection);
+
 public:
   Document* m_document;
   Glib::ustring m_table_name;
+  type_map_field_values m_map_field_values;
+private:
   sharedptr<const Field> m_key_field;
   Gnome::Gda::Value m_key_field_value;
 
   boost::python::object m_related; //Actually a PyGlomRelated
 
-  //Available, for instance, in python via record["name_first"]
-  typedef std::map<Glib::ustring, Gnome::Gda::Value> type_map_field_values;
-  type_map_field_values m_map_field_values;
-
   Glib::RefPtr<Gnome::Gda::Connection> m_connection;
   
 private:
   bool m_read_only;
 };
 
-void PyGlomRecord_SetFields(PyGlomRecord* self,
-  const PyGlomRecord::type_map_field_values& field_values,
-  Document* document,
-  const Glib::ustring& table_name,
-  const sharedptr<const Field>& key_field,
-  const Gnome::Gda::Value& key_field_value,
-  const Glib::RefPtr<Gnome::Gda::Connection>& opened_connection);
+
 
 } //namespace Glom
 
diff --git a/glom/libglom/python_embed/py_glom_related.cc b/glom/libglom/python_embed/py_glom_related.cc
index 1676d38..d25a49a 100644
--- a/glom/libglom/python_embed/py_glom_related.cc
+++ b/glom/libglom/python_embed/py_glom_related.cc
@@ -96,7 +96,7 @@ boost::python::object PyGlomRelated::getitem(const boost::python::object& cppite
                 if(!Conversions::value_is_empty(from_key_value)) //Do not link on null-values. That would cause us to link on 0, or "0".
                   key_value_sqlized = from_key_field->sql(from_key_value);
 
-                PyGlomRelatedRecord_SetRelationship(pyRelatedRecord, iterFind->second, key_value_sqlized, record->m_document);
+                pyRelatedRecord->set_relationship(iterFind->second, key_value_sqlized, record->m_document);
 
                 //Store it in the cache:
                 boost::python::object objectRelatedRecord(pyRelatedRecord);
@@ -126,9 +126,9 @@ static void Related_HandlePythonError()
 */
 
 
-void PyGlomRelated_SetRelationships(PyGlomRelated* self, const PyGlomRelated::type_map_relationships& relationships)
+void PyGlomRelated::set_relationships(const PyGlomRelated::type_map_relationships& relationships)
 {
-  self->m_map_relationships = relationships;
+  m_map_relationships = relationships;
 }
 
 } //namespace Glom
diff --git a/glom/libglom/python_embed/py_glom_related.h b/glom/libglom/python_embed/py_glom_related.h
index 83e33a6..b2b024c 100644
--- a/glom/libglom/python_embed/py_glom_related.h
+++ b/glom/libglom/python_embed/py_glom_related.h
@@ -36,16 +36,19 @@ public:
   PyGlomRelated();
   ~PyGlomRelated();
 
+  typedef std::map<Glib::ustring, sharedptr<Relationship> > type_map_relationships;
+  void set_relationships(const PyGlomRelated::type_map_relationships& relationships);
+
+
   //[] notation:
   long len() const;
   boost::python::object getitem(const boost::python::object& item);
 
   friend class PyGlomRecord;
 
-  typedef std::map<Glib::ustring, sharedptr<Relationship> > type_map_relationships;
+private:
   typedef std::map<Glib::ustring, boost::python::object /* Actually PyGlomRelatedRecord* */> type_map_relatedrecords;
 
-//TODO: protected:
   boost::python::object m_record; //Actually PyGlomRecord. A reference to the parent record.
 
 
@@ -54,7 +57,6 @@ public:
   type_map_relatedrecords m_map_relatedrecords;
 };
 
-void PyGlomRelated_SetRelationships(PyGlomRelated* self, const PyGlomRelated::type_map_relationships& relationships);
 
 
 } //namespace Glom
diff --git a/glom/libglom/python_embed/py_glom_relatedrecord.cc b/glom/libglom/python_embed/py_glom_relatedrecord.cc
index 7d7c23f..48d2bca 100644
--- a/glom/libglom/python_embed/py_glom_relatedrecord.cc
+++ b/glom/libglom/python_embed/py_glom_relatedrecord.cc
@@ -248,13 +248,11 @@ boost::python::object PyGlomRelatedRecord::max(const std::string& field_name) co
   return generic_aggregate(field_name, "max");
 }
 
-void PyGlomRelatedRecord_SetRelationship(PyGlomRelatedRecord* self, const sharedptr<const Relationship>& relationship, const Glib::ustring& from_key_value_sqlized,  Document* document)
+void PyGlomRelatedRecord::set_relationship(const sharedptr<const Relationship>& relationship, const Glib::ustring& from_key_value_sqlized,  Document* document)
 {
-  self->m_relationship = relationship;
-
-  self->m_from_key_value_sqlized = from_key_value_sqlized;
-
-  self->m_document = document;
+  m_relationship = relationship;
+  m_from_key_value_sqlized = from_key_value_sqlized;
+  m_document = document;
 }
 
 } //namespace Glom
diff --git a/glom/libglom/python_embed/py_glom_relatedrecord.h b/glom/libglom/python_embed/py_glom_relatedrecord.h
index c4dad58..918794f 100644
--- a/glom/libglom/python_embed/py_glom_relatedrecord.h
+++ b/glom/libglom/python_embed/py_glom_relatedrecord.h
@@ -38,6 +38,8 @@ public:
   PyGlomRelatedRecord();
   ~PyGlomRelatedRecord();
 
+  void set_relationship(const sharedptr<const Relationship>& relationship, const Glib::ustring& from_key_value_sqlized, Document* document);
+
   boost::python::object sum(const std::string& field_name) const;
   boost::python::object count(const std::string& field_name) const;
   boost::python::object min(const std::string& field_name) const;
@@ -47,7 +49,7 @@ public:
   long len() const;
   boost::python::object getitem(const boost::python::object& item);
 
-//TODO: protected:
+private:
 
   boost::python::object generic_aggregate(const std::string& field_name, const std::string& aggregate) const;
 
@@ -63,8 +65,6 @@ public:
   mutable type_map_field_values m_map_field_values; //A cache.
 };
 
-void PyGlomRelatedRecord_SetRelationship(PyGlomRelatedRecord* self, const sharedptr<const Relationship>& relationship, const Glib::ustring& from_key_value_sqlized, Document* document);
-
 /*
 void PyGlomRelatedRecord_SetConnection(PyGlomRelatedRecord* self, const Glib::RefPtr<Gnome::Gda::Connection>& connection);
 */
diff --git a/glom/libglom/python_embed/pygdavalue_conversions.cc b/glom/libglom/python_embed/pygdavalue_conversions.cc
index 1fd5c76..9078fa4 100644
--- a/glom/libglom/python_embed/pygdavalue_conversions.cc
+++ b/glom/libglom/python_embed/pygdavalue_conversions.cc
@@ -119,6 +119,8 @@ glom_pygda_value_from_pyobject(GValue* boxed, const boost::python::object& input
          gda_value_set_time (boxed, &gda);
          return true;
      }
+#else
+  //std::cout << "DEBUG Dates not supported." << std::endl;
 #endif
 
     //g_warning("Unhandled python type.");
@@ -145,19 +147,32 @@ boost::python::object glom_pygda_value_as_boost_pyobject(const Glib::ValueBase&
         ret = boost::python::object((bool)g_value_get_boolean(boxed));
 #if PY_VERSION_HEX >= 0x02040000
     } else if (value_type == G_TYPE_DATE) {
-        /* TODO: 
+                  
         const GDate* val = (const GDate*)g_value_get_boxed(boxed);
         if(val)
-          ret = PyDate_FromDate(val->year, val->month, val->day);
-        */
+        {
+          //Note that the g_date_get* functions give what we expect, but direct struct field access does not.
+          const int year = g_date_get_year(val);
+          const int month = g_date_get_month(val);
+          const int day = g_date_get_day(val);
+
+          if(!g_date_valid(val))
+            std::cerr << "glom_pygda_value_as_boost_pyobject(): The GDate is not valid." << std::endl;
+            
+          //std::cout << "DEBUG G_TYPE_DATE: year=" << year << ", month=" << month << ", day=" << day << std::endl;
+          PyObject* cobject = PyDate_FromDate(year, month, day);
+          ret = boost::python::object( (boost::python::handle<>(cobject)) );
+        }
 #endif
     } else if (value_type == G_TYPE_DOUBLE) {
         ret = boost::python::object(g_value_get_double(boxed));
     } else if (value_type == GDA_TYPE_GEOMETRIC_POINT) {
-    /*
-        const GdaGeometricPoint* val = gda_value_get_geometric_point(boxed;
-        ret = Py_BuildValue ("(ii)", val->x, val->y);
-    */
+        const GdaGeometricPoint* val = gda_value_get_geometric_point(boxed);
+        if(val)
+        {
+          PyObject* cobject = Py_BuildValue ("(ii)", val->x, val->y);
+          ret = boost::python::object( (boost::python::handle<>(cobject)) );
+        }
     } else if (value_type == G_TYPE_INT) {
         ret = boost::python::object(g_value_get_int(boxed));
     } else if (value_type == GDA_TYPE_NUMERIC) {
@@ -174,11 +189,19 @@ boost::python::object glom_pygda_value_as_boost_pyobject(const Glib::ValueBase&
         ret = boost::python::object(val);
     } else if (value_type == GDA_TYPE_TIME) {
 #if PY_VERSION_HEX >= 0x02040000
-        //const GdaTime* val = gda_value_get_time(boxed)
-        //ret = PyTime_FromTime(val->hour, val->minute, val->second, 0); /* TODO: Should we ignore GDate::timezone ? */
+        const GdaTime* val = gda_value_get_time(boxed);
+        if(val)
+        {
+          PyObject* cobject = PyTime_FromTime(val->hour, val->minute, val->second, 0); /* TODO: Should we ignore GDate::timezone ? */
+          ret = boost::python::object( (boost::python::handle<>(cobject)) );
+        }
     } else if (value_type == GDA_TYPE_TIMESTAMP) {
-        //const GdaTimestamp* val = gda_value_get_timestamp (boxed;
-        //ret = PyDateTime_FromDateAndTime(val->year, val->month, val->day, val->hour, val->minute, val->second, 0); /* TODO: Should we ignore GdaTimestamp::timezone ? */
+        const GdaTimestamp* val = gda_value_get_timestamp(boxed);
+        if(val)
+        {
+          PyObject* cobject = PyDateTime_FromDateAndTime(val->year, val->month, val->day, val->hour, val->minute, val->second, 0); /* TODO: Should we ignore GdaTimestamp::timezone ? */
+          ret = boost::python::object( (boost::python::handle<>(cobject)) );
+        }
 #endif
     } else if (value_type == GDA_TYPE_SHORT) {
         ret = boost::python::object(gda_value_get_short(boxed));
diff --git a/glom/python_embed/glom_python.cc b/glom/python_embed/glom_python.cc
index 944dfd2..bdcebd3 100644
--- a/glom/python_embed/glom_python.cc
+++ b/glom/python_embed/glom_python.cc
@@ -387,7 +387,7 @@ void glom_execute_python_function_implementation(const Glib::ustring& func_impl,
   if(pParam)
   {
     //Fill the record's details:
-    PyGlomRecord_SetFields(pParam, field_values, pDocument, table_name, key_field, key_field_value, opened_connection);
+    pParam->set_fields(field_values, pDocument, table_name, key_field, key_field_value, opened_connection);
     pParam->set_read_only();
   }
   
@@ -427,7 +427,7 @@ Gnome::Gda::Value glom_evaluate_python_function_implementation(Field::glom_field
   if(pParam)
   {
     //Fill the record's details:
-    PyGlomRecord_SetFields(pParam, field_values, pDocument, table_name, key_field, key_field_value, opened_connection);
+    pParam->set_fields(field_values, pDocument, table_name, key_field, key_field_value, opened_connection);
   }
   
   const boost::python::object pyResultCpp = glom_python_call(result_type, pDocument, func_impl, objRecord);
diff --git a/tests/test_python_execute_func_date.cc b/tests/test_python_execute_func_date.cc
index ba3b32c..f72ee0a 100644
--- a/tests/test_python_execute_func_date.cc
+++ b/tests/test_python_execute_func_date.cc
@@ -34,15 +34,6 @@ void execute_func_with_date_return_value()
 
   //std::cout << "value=" << value.to_string() << std::endl;
 }
-
-/*
-TODO: Test this too:
-"from dateutil.relativedelta import relativedelta\n"
-                            "import datetime\n"
-                            "today = datetime.date.today()\n"
-                            "date_of_birth = record[\"test_field\"]
-                            "rd = relativedelta(today, date_of_birth)
-*/
                             
 void execute_func_with_date_input_value()
 {
@@ -74,12 +65,49 @@ void execute_func_with_date_input_value()
   //std::cout << "value=" << value.to_string() << std::endl;
 }
 
+/* This would require the extra dateutil python module.
+void execute_func_with_date_input_value_relativedelta()
+{
+  const char* calculation = "from dateutil.relativedelta import relativedelta\n"
+                            "import datetime\n"
+                            "today = datetime.date.today()\n"
+                            "date_of_birth = record[\"test_field\"]\n"
+                            "rd = relativedelta(today, date_of_birth)\n"
+                            "return rd.year";
+  Glom::type_map_fields field_values;
+  const Glib::Date input_date = Glib::Date(11, Glib::Date::MAY, 1973);
+  field_values["test_field"] = Gnome::Gda::Value(input_date);
+  Glib::RefPtr<Gnome::Gda::Connection> connection;
+
+  //Execute a python function:
+  const Gnome::Gda::Value value = Glom::glom_evaluate_python_function_implementation(
+    Glom::Field::TYPE_NUMERIC, calculation, field_values,
+    0, "",
+    Glom::sharedptr<Glom::Field>(), Gnome::Gda::Value(), // primary key details. Not used in this test.
+    connection);
+
+  //std::cout << "type=" << g_type_name(value.get_value_type()) << std::endl;
+
+  //Check that the return value is of the expected type:
+  g_assert(value.get_value_type() == GDA_TYPE_NUMERIC);
+
+  //Check that the return value is of the expected value:
+  g_assert(value.get_numeric());
+  g_assert(value.get_numeric()->number);
+  //std::cout << "GdaNumeric number=" << value.get_numeric()->number << std::endl;
+  //g_assert(value.get_numeric()->number == std::string("1973"));
+
+  std::cout << "value=" << value.to_string() << std::endl;
+}
+*/
+
 int main()
 {
   Glom::libglom_init(); //Also initializes python.
 
   execute_func_with_date_return_value();
   execute_func_with_date_input_value();
+  //execute_func_with_date_input_value_relativedelta();
 
   return EXIT_SUCCESS;
 }