[Rprotobuf-commits] r501 - in pkg: . R inst/unitTests man src vignettes/RProtoBuf

noreply at r-forge.r-project.org noreply at r-forge.r-project.org
Fri Jul 12 09:54:04 CEST 2013


Author: murray
Date: 2013-07-12 09:54:04 +0200 (Fri, 12 Jul 2013)
New Revision: 501

Modified:
   pkg/ChangeLog
   pkg/R/extensions.R
   pkg/inst/unitTests/runit.extensions.R
   pkg/man/Message-class.Rd
   pkg/src/extensions.cpp
   pkg/src/extractors.cpp
   pkg/src/mutators.cpp
   pkg/src/rprotobuf.cpp
   pkg/src/rprotobuf.h
   pkg/vignettes/RProtoBuf/RProtoBuf.Rnw
Log:
Further improve, document, and test the new extensions support.  Specifically,
we now reuse the extractor and mutator methods for normal fields for
extensions as well so we don't have to duplicate any type handling.



Modified: pkg/ChangeLog
===================================================================
--- pkg/ChangeLog	2013-07-11 08:05:00 UTC (rev 500)
+++ pkg/ChangeLog	2013-07-12 07:54:04 UTC (rev 501)
@@ -1,3 +1,18 @@
+2013-07-12  Murray Stokely  <murray at FreeBSD.org>
+
+	* src/extensions.cpp: Replace custom extractor and mutator with
+	  call to existing functions in mutators.cpp and extractors.cpp that
+	  can be used for extensions and any other field type.
+	* src/extensions.R: Idem
+	* vignettes/RProtoBuf/RProtoBuf.Rnw: Document getExtension,
+	  setExtension methods.
+	* man/Message-class.Rd: Idem
+	* src/rprotobuf.h: Remove unused desc argument to
+	  extractFieldAsSEXP.
+	* src/extractors.cpp: Idem
+	* inst/unitTests/runit.extensions.R: Add additional tests for
+	  nested enum and message type extensions.
+
 2013-07-11  Murray Stokely  <murray at FreeBSD.org>
 
 	* R/extensions.R: Implement getExtension, setExtension methods.

Modified: pkg/R/extensions.R
===================================================================
--- pkg/R/extensions.R	2013-07-11 08:05:00 UTC (rev 500)
+++ pkg/R/extensions.R	2013-07-12 07:54:04 UTC (rev 501)
@@ -23,19 +23,8 @@
 setMethod( "setExtension", "Message", function( object, field, values ){
 	stopifnot(is_extension(field))
 
-	if( is( values, "Message" ) ){
-		values <- list( values )
-	}
-
-        if (is_repeated(field)) {
-            .Call( "setExtensionRepeatedField", object at pointer, field, values,
-                  PACKAGE = "RProtoBuf" )
-        } else {
-            ## this check only makes sense for vector types.
-            stopifnot(length(values) == 1)
-            .Call( "setExtensionField", object at pointer, field, values,
-                  PACKAGE = "RProtoBuf" )
-        }
+        .Call( "setMessageField", object at pointer, field, values,
+              PACKAGE = "RProtoBuf" )
 	invisible( object )
 } )
 
@@ -44,11 +33,6 @@
 	standardGeneric( "getExtension" )
 } )
 setMethod( "getExtension", "Message", function( object, field){
-	if (is_repeated(field)) {
-		.Call( "getExtensionRepeated", object at pointer, field,
-		      PACKAGE = "RProtoBuf" )
-	} else {
-		.Call( "getExtension", object at pointer, field,
-		      PACKAGE = "RProtoBuf" )
-	}
+        .Call( "getExtension", object at pointer, field,
+              PACKAGE = "RProtoBuf" )
 } )

Modified: pkg/inst/unitTests/runit.extensions.R
===================================================================
--- pkg/inst/unitTests/runit.extensions.R	2013-07-11 08:05:00 UTC (rev 500)
+++ pkg/inst/unitTests/runit.extensions.R	2013-07-12 07:54:04 UTC (rev 501)
@@ -44,4 +44,25 @@
     checkEquals(test$getExtension(protobuf_unittest.repeated_int32_extension),
                 1:10)
 
+    ## Test nested extensions.
+    test$setExtension(protobuf_unittest.TestNestedExtension.test, "foo")
+    checkEquals(test$getExtension(protobuf_unittest.TestNestedExtension.test),
+                                  "foo")
+
+    ## Test setting and getting enums.
+    # This works now
+    test$setExtension(protobuf_unittest.optional_nested_enum_extension,
+                      protobuf_unittest.TestAllTypes.NestedEnum$BAR)
+
+    # Test that we get an error printed to terminal (not a real stop error)
+    # not a crash for invalid enum:
+    # TODO(mstokely): Make this a stop() error.
+    test$setExtension(protobuf_unittest.optional_nested_enum_extension,
+                      9)
+    
+    ## Test nested message extensions.
+    tmp <- new( protobuf_unittest.TestAllTypes.NestedMessage )
+    tmp$bb <- 3
+    test$setExtension(protobuf_unittest.optional_nested_message_extension, tmp)
+    checkEquals(test$getExtension(protobuf_unittest.optional_nested_message_extension)$bb, 3)
 }

Modified: pkg/man/Message-class.Rd
===================================================================
--- pkg/man/Message-class.Rd	2013-07-11 08:05:00 UTC (rev 500)
+++ pkg/man/Message-class.Rd	2013-07-12 07:54:04 UTC (rev 501)
@@ -55,6 +55,10 @@
     \item{length}{\code{signature(x = "Message")}: The number of fields actually contained in the message. 
     A field counts in these two situations: the field is repeated and the field size is greater than 0, 
     the field is not repeated and the message has the field.}
+    \item{setExtension}{\code{signature(object = "Message")}: set an
+      extension field of the Message.}
+    \item{getExtension}{\code{signature(object = "Message")}: get the
+      value of an extension field of the Message.}
     \item{str}{\code{signature(object = "Message")}: displays the structure of the message }
     \item{identical}{\code{signature(x = "Message", y = "Message")}: Test if two messages are exactly identical }
     \item{==}{\code{signature(e1 = "Message", e2 = "Message")}: Same as \code{identical} }

Modified: pkg/src/extensions.cpp
===================================================================
--- pkg/src/extensions.cpp	2013-07-11 08:05:00 UTC (rev 500)
+++ pkg/src/extensions.cpp	2013-07-12 07:54:04 UTC (rev 501)
@@ -28,239 +28,30 @@
 // TODO add num_extensions to wrapper_Message.cpp and show this also
 // in show().
 
-
-/**
- * set a non-repeated extension field to a new value
- *
- * @param pointer external pointer to a message
- * @param sfielddesc S4 field descriptor class
- * @param value new value for the field
- *
- * @return always NULL, the message is modified by reference
- */
-RcppExport SEXP setExtensionField( SEXP pointer, SEXP sfielddesc, SEXP value ){
-        GPB::Message* message = GET_MESSAGE_POINTER_FROM_XP(pointer) ;
-        const Reflection * ref = message->GetReflection() ;
-        GPB::FieldDescriptor* field_desc = GET_FIELD_DESCRIPTOR_POINTER_FROM_S4(sfielddesc);
-
-	if (!field_desc->is_extension()) {
-		Rprintf("Field is not an extension! tag = %d type = %s\n",
-			field_desc->number(), field_desc->type_name());
-		return R_NilValue;
-	}
-
-	switch (field_desc->type()) {
-		// All floating-point and 64-bit types as R numerics.
-		case TYPE_DOUBLE:
-			ref->SetDouble(message, field_desc, REAL(value)[0]);
-			break;
-		case TYPE_FLOAT:
-			ref->SetFloat(message, field_desc, REAL(value)[0]);
-			break;
-		case TYPE_INT64:
-			ref->SetInt64(message, field_desc, REAL(value)[0]);
-			break;
-		case TYPE_UINT64:
-			ref->SetUInt64(message, field_desc, REAL(value)[0]);
-			break;
-		case TYPE_INT32:
-			ref->SetInt32(message, field_desc, INTEGER(value)[0]);
-			break;
-		case TYPE_UINT32:
-			ref->SetUInt32(message, field_desc, INTEGER(value)[0]);
-			break;
-		case TYPE_BOOL:
-			ref->SetBool(message, field_desc, LOGICAL(value)[0]);
-			break;
-		case TYPE_STRING:
-			ref->SetString(message, field_desc,
-				       CHAR(STRING_ELT(value, 0)));
-			break;
-
-// TODO(mstokely): support enums and messages.
-// Check value is an EnumValueDescriptor first.
-//		case TYPE_ENUM:
-//			ref->SetEnum(message, field_desc, value);
-//			break;
-		default:
-			printf("Not implemented yet");
-	}
-
-	return R_NilValue;
-}
-
-/**
- * set a repeated extension field to a new value
- *
- * @param pointer external pointer to a message
- * @param sfielddesc S4 field descriptor class
- * @param value new value for the field
- *
- * @return always NULL, the message is modified by reference
- */
-RcppExport SEXP setExtensionRepeatedField( SEXP pointer,
-					   SEXP sfielddesc, SEXP value ){
-        GPB::Message* message = GET_MESSAGE_POINTER_FROM_XP(pointer) ;
-        const Reflection * ref = message->GetReflection() ;
-        GPB::FieldDescriptor* field_desc = GET_FIELD_DESCRIPTOR_POINTER_FROM_S4(sfielddesc);
-
-	if (!field_desc->is_extension()) {
-		Rprintf("Field is not an extension! tag = %d type = %s\n",
-			field_desc->number(), field_desc->type_name());
-		return R_NilValue;
-	}
-
-	ref->ClearField(message, field_desc);
-	Rprintf("in set repeated extension: length: %d\n", LENGTH(value));
-	switch (field_desc->type()) {		// All floating-point and 64-bit types as R numerics.
-		case TYPE_DOUBLE:
-			for (int i = 0; i < LENGTH(value); i++) {
-				ref->AddDouble(message, field_desc,
-					       REAL(value)[i]);
-			}
-			break;
-		case TYPE_FLOAT:
-			for (int i = 0; i < LENGTH(value); i++) {
-				ref->AddFloat(message, field_desc,
-						      REAL(value)[i]);
-			}
-			break;
-		case TYPE_INT64:
-			for (int i = 0; i < LENGTH(value); i++) {
-				ref->AddInt64(message, field_desc,
-					      REAL(value)[i]);
-			}
-			break;
-		case TYPE_UINT64:
-			for (int i = 0; i < LENGTH(value); i++) {
-				  ref->AddUInt64(message, field_desc,
-						 REAL(value)[i]);
-			}
-			break;
-		case TYPE_INT32:
-			for (int i = 0; i < LENGTH(value); i++) {
-				ref->AddInt32(message, field_desc,
-					      INTEGER(value)[i]);
-			}
-			break;
-		case TYPE_UINT32:
-			for (int i = 0; i < LENGTH(value); i++) {
-				ref->AddUInt32(message, field_desc,
-					       INTEGER(value)[i]);
-			}
-			break;
-		case TYPE_BOOL:
-			for (int i = 0; i < LENGTH(value); i++) {
-				ref->AddBool(message, field_desc,
-					     LOGICAL(value)[i]);
-			}
-			break;
-		case TYPE_STRING:
-			for (int i = 0; i < LENGTH(value); i++) {
-				ref->AddString(message, field_desc,
-					       CHAR(STRING_ELT(value, i)));
-			}
-			break;
-// TODO(mstokely): support enums, messages.
-// Check value is an EnumValueDescriptor first.
-//		case TYPE_ENUM:
-//			ref->SetEnum(message, field_desc, value);
-//			break;
-		default:
-			printf("Not implemented yet");
-	}
-
-	return R_NilValue;
-}
-
 RcppExport SEXP getExtension( SEXP pointer, SEXP sfielddesc){
         /* grab the Message pointer */
-        const GPB::Message* message = GET_MESSAGE_POINTER_FROM_XP(pointer) ;
-        const Reflection * ref = message->GetReflection() ;
-        GPB::FieldDescriptor* field_desc =
+	Rcpp::XPtr<GPB::Message> message(pointer) ;
+	const Reflection * ref = message->GetReflection() ;
+        const GPB::FieldDescriptor* field_desc =
 		GET_FIELD_DESCRIPTOR_POINTER_FROM_S4(sfielddesc);
 
-        if (!field_desc->is_extension()) {
-		Rprintf("Field is not an extension! tag = %d type = %s\n",
-			field_desc->number(), field_desc->type_name());
-		return R_NilValue;
-        }
-	if (!ref->HasField(*message, field_desc)) {
-		return R_NilValue;
-        }
+	// extractFieldAsSEXP returns a default (e.g. 0) even when
+	// field doesn't exist, but returning NULL probably makes more
+	// sense.
+	//
+	// TODO(mstokely): move this logic into extractField so that
+	// all fields get this updated behavior, not just extensions.
 
-	switch (field_desc->type()) {
-          case TYPE_DOUBLE:
-		return(Rcpp::wrap( ref->GetDouble(*message, field_desc) ));
-          case TYPE_FLOAT:
-		return(Rcpp::wrap( ref->GetFloat(*message, field_desc) ));
-          case TYPE_INT64:
-		return(Rcpp::wrap( ref->GetInt64(*message, field_desc) ));
-          case TYPE_UINT64:
-		return(Rcpp::wrap( ref->GetUInt64(*message, field_desc) ));
-          case TYPE_INT32:
-		return(Rcpp::wrap( ref->GetInt32(*message, field_desc) ));
-          case TYPE_UINT32:
-		return(Rcpp::wrap( ref->GetUInt32(*message, field_desc) ));
-          case TYPE_BOOL:
-		return(Rcpp::wrap( ref->GetBool(*message, field_desc) ));
-          case TYPE_STRING:
-		return(Rcpp::wrap( ref->GetString(*message, field_desc) ));
-          default:
-		printf("Not implemented yet");
-        }
-        return R_NilValue;
+	if (field_desc->is_repeated()) {
+	  if (ref->FieldSize(*message, field_desc) < 1) {
+	    return R_NilValue;
+	  }
+	} else {
+	  if (!ref->HasField(*message, field_desc)) {
+	    return R_NilValue;
+	  }
+	}
+	return( extractFieldAsSEXP(message, field_desc) );
 }
 
-RcppExport SEXP getExtensionRepeated( SEXP pointer, SEXP sfielddesc){
-        /* grab the Message pointer */
-        const GPB::Message* message = GET_MESSAGE_POINTER_FROM_XP(pointer) ;
-        const Reflection * ref = message->GetReflection() ;
-        GPB::FieldDescriptor* field_desc =
-		GET_FIELD_DESCRIPTOR_POINTER_FROM_S4(sfielddesc);
-
-        if (!field_desc->is_extension()) {
-		Rprintf("Field is not an extension! tag = %d type = %s\n",
-			field_desc->number(), field_desc->type_name());
-		return R_NilValue;
-        }
-	if (ref->FieldSize(*message, field_desc) < 1) {
-		return R_NilValue;
-        }
-
-    	switch( GPB::FieldDescriptor::TypeToCppType(field_desc->type()) ){
-	  case CPPTYPE_DOUBLE:
-			return Rcpp::wrap( RepeatedFieldImporter<double>(ref,
-								 *message,
-								 field_desc));
-	  case CPPTYPE_FLOAT:
-			return Rcpp::wrap( RepeatedFieldImporter<float>(ref,
-								 *message,
-								 field_desc));
-  	  case CPPTYPE_INT32:
-		return Rcpp::wrap( RepeatedFieldImporter<int32>(ref,
-								*message,
-								field_desc));
-	  case CPPTYPE_UINT32:
-		return Rcpp::wrap( RepeatedFieldImporter<uint32>(ref,
-								*message,
-								field_desc));
-	  case CPPTYPE_INT64:
-		return Rcpp::wrap( RepeatedFieldImporter<int64>(ref,
-								*message,
-								field_desc));
-	  case CPPTYPE_UINT64:
-		return Rcpp::wrap( RepeatedFieldImporter<uint64>(ref,
-								*message,
-								field_desc));
-	  case CPPTYPE_BOOL:
-		return Rcpp::wrap( RepeatedFieldImporter<bool>(ref,
-							       *message,
-							       field_desc));
-          default:
-		printf("Not implemented yet");
-        }
-        return R_NilValue;
-}
-
 }  // namespace rprotobuf

Modified: pkg/src/extractors.cpp
===================================================================
--- pkg/src/extractors.cpp	2013-07-11 08:05:00 UTC (rev 500)
+++ pkg/src/extractors.cpp	2013-07-12 07:54:04 UTC (rev 501)
@@ -45,20 +45,18 @@
 	/* grab the Message pointer */
 	Rcpp::XPtr<GPB::Message> message(pointer) ;
 
-	/* the message descriptor */
-	const GPB::Descriptor* desc = message->GetDescriptor() ;
-	
 	GPB::FieldDescriptor* field_desc = getFieldDescriptor( message, name ) ;
 	
 #ifdef RPB_DEBUG
 Rprintf( "</getMessageField>\n" ) ;
 #endif
 
-	return( extractFieldAsSEXP(message, desc, field_desc) ) ;
+	return( extractFieldAsSEXP(message, field_desc) ) ;
 	
 }
 
-SEXP extractFieldAsSEXP( const Rcpp::XPtr<GPB::Message>& message, const GPB::Descriptor* desc, const GPB::FieldDescriptor*  fieldDesc ){
+SEXP extractFieldAsSEXP( const Rcpp::XPtr<GPB::Message>& message,
+						 const GPB::FieldDescriptor*  fieldDesc ){
 
     const Reflection * ref = message->GetReflection() ;
        

Modified: pkg/src/mutators.cpp
===================================================================
--- pkg/src/mutators.cpp	2013-07-11 08:05:00 UTC (rev 500)
+++ pkg/src/mutators.cpp	2013-07-12 07:54:04 UTC (rev 501)
@@ -353,7 +353,7 @@
  * set a message field to a new value
  *
  * @param pointer external pointer to a message
- * @param name name of the field
+ * @param name name of the field, or an S4 FieldDescriptor object.
  * @param value new value for the field
  *
  * @return allways NULL, the message is modified by reference

Modified: pkg/src/rprotobuf.cpp
===================================================================
--- pkg/src/rprotobuf.cpp	2013-07-11 08:05:00 UTC (rev 500)
+++ pkg/src/rprotobuf.cpp	2013-07-12 07:54:04 UTC (rev 501)
@@ -190,6 +190,15 @@
 	GPB::FieldDescriptor* field_desc = (GPB::FieldDescriptor*)0;
 	const GPB::Descriptor* desc = message->GetDescriptor() ;
 	switch( TYPEOF(name) ){
+		case S4SXP:
+		  {
+		    if (Rf_inherits( name, "FieldDescriptor") ){
+		      field_desc = GET_FIELD_DESCRIPTOR_POINTER_FROM_S4(name);
+		    } else {
+		      throwException( "S4 class is not a FieldDescriptor", "NoSuchFieldException" ) ;
+		    }
+		    break ;
+		  }
 		case CHARSXP:
 			{
 				field_desc = (GPB::FieldDescriptor*)desc->FindFieldByName( CHAR(name) ) ;

Modified: pkg/src/rprotobuf.h
===================================================================
--- pkg/src/rprotobuf.h	2013-07-11 08:05:00 UTC (rev 500)
+++ pkg/src/rprotobuf.h	2013-07-12 07:54:04 UTC (rev 501)
@@ -94,6 +94,9 @@
 #define GET_FIELD_DESCRIPTOR_POINTER_FROM_XP(xp)  (GPB::FieldDescriptor*) EXTPTR_PTR( xp )
 #define GET_FIELD_DESCRIPTOR_POINTER_FROM_S4(m)   (GPB::FieldDescriptor*) EXTPTR_PTR( GET_SLOT( m, Rf_install("pointer") ) )
 
+#define GET_ENUM_VALUE_DESCRIPTOR_POINTER_FROM_XP(xp)  (GPB::EnumValueDescriptor*) EXTPTR_PTR( xp )
+#define GET_ENUM_VALUE_DESCRIPTOR_POINTER_FROM_S4(m)   (GPB::EnumValueDescriptor*) EXTPTR_PTR( GET_SLOT( m, Rf_install("pointer") ) )
+
 #define GET_METHOD(xp)  (GPB::MethodDescriptor*) EXTPTR_PTR( xp )
 
 #define COPYSTRING(s) s
@@ -123,7 +126,7 @@
 
 /* in extractors.cpp */
 RcppExport SEXP getMessageField( SEXP, SEXP ); 
-RcppExport SEXP extractFieldAsSEXP( const Rcpp::XPtr<GPB::Message>& , const GPB::Descriptor*, const GPB::FieldDescriptor* ) ;
+RcppExport SEXP extractFieldAsSEXP( const Rcpp::XPtr<GPB::Message>& , const GPB::FieldDescriptor* ) ;
 
 /* in exceptions.cpp */
 RcppExport SEXP throwException( const char*, const char*) ;

Modified: pkg/vignettes/RProtoBuf/RProtoBuf.Rnw
===================================================================
--- pkg/vignettes/RProtoBuf/RProtoBuf.Rnw	2013-07-11 08:05:00 UTC (rev 500)
+++ pkg/vignettes/RProtoBuf/RProtoBuf.Rnw	2013-07-12 07:54:04 UTC (rev 501)
@@ -179,12 +179,23 @@
 p[[ "email" ]]
 @
 
+% TODO(mstokely): Document extensions here.
+% There are none in addressbook.proto though.
+
 \subsection{Display messages}
 
-For debugging purposes, protocol buffer messages
-implement the \texttt{as.character} method:
+Protocol buffer messages and descriptors implement \texttt{show}
+methods that provide basic information about the message :
 
 <<>>=
+p
+@
+
+For additional information, such as for debugging purposes,
+the \texttt{as.character} method provides a more complete ASCII
+representation of the contents of a message.
+
+<<>>=
 writeLines( as.character( p ) )
 @
 
@@ -355,7 +366,9 @@
 \texttt{swap} & \ref{Message-method-swap} & swap elements of a repeated field of a message\\
 \texttt{set} & \ref{Message-method-set} & set elements of a repeated field\\
 \texttt{fetch} & \ref{Message-method-fetch} & fetch elements of a repeated field\\
-\texttt{add} & \label{Message-method-add} & add elements to a repeated field \\
+\texttt{setExtension} & \ref{Message-method-setExtension} & set an extension of a message\\
+\texttt{getExtension} & \ref{Message-method-getExtension} & get the value of an extension of a message\\
+\texttt{add} & \ref{Message-method-add} & add elements to a repeated field \\
 \hline
 \texttt{str} & \ref{Message-method-str} & the R structure of the message\\
 \texttt{as.character} & \ref{Message-method-ascharacter} & character representation of a message\\
@@ -664,7 +677,7 @@
 \subsubsection{Message\$fetch method}
 \label{Message-method-fetch}
 
-The \texttt{fetch} method can be used to set values
+The \texttt{fetch} method can be used to get values
 of a repeated field.
 
 <<keep.source=T>>=
@@ -676,6 +689,37 @@
 message$fetch( "phone", 1 )
 @
 
+\subsubsection{Message\$setExtension method}
+\label{Message-method-setExtension}
+
+The \texttt{setExtension} method can be used to get values
+of a repeated field.
+
+<<keep.source=T>>=
+if (!exists("protobuf_unittest.TestAllTypes",
+            "RProtoBuf:DescriptorPool")) {
+    unittest.proto.file <- system.file("unitTests", "data",
+                                       "unittest.proto",
+                                       package="RProtoBuf")
+    readProtoFiles(file=unittest.proto.file)
+}
+
+## Test setting a singular extensions.
+test <- new(protobuf_unittest.TestAllExtensions)
+test$setExtension(protobuf_unittest.optional_int32_extension,
+                  as.integer(1))
+@ 
+
+\subsubsection{Message\$getExtension method}
+\label{Message-method-getExtension}
+
+The \texttt{getExtension} method can be used to get values
+of an extension.
+
+<<keep.source=T>>=
+test$getExtension(protobuf_unittest.optional_int32_extension)
+@
+
 \subsubsection{Message\$add method}
 \label{Message-method-add}
 
@@ -952,8 +996,15 @@
 
 \subsubsection{name}
 
-TODO
+The \texttt{name} method can be used to retrieve the name of the
+field descriptor.
 
+<<>>=
+# simple name.
+name( tutorial.Person$id )
+# name including scope.
+name( tutorial.Person$id, full=TRUE )
+@
 
 \subsection{enum descriptors}
 \label{subsec-enum-descriptor}
@@ -1117,6 +1168,7 @@
 \item field names for messages
 \item field names, enum types, nested types for message type descriptors
 \item names for enum descriptors
+\item names for top-level extensions
 \end{itemize}
 
 \subsection{with and within}



More information about the Rprotobuf-commits mailing list