----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/12253/#review22798 -----------------------------------------------------------
I'm ok with this approach (although I suggested an alternative to you offline, which is to use a Python wrapper class). I'm concerned with conversions that have loss of precision or possibility of overflow, like double->float, long->int, int->byte, etc. What's the reason to have them? Maybe you should have separate interfaces for converting from Java to Writable and back, so that we only allow the correct directions. E.g. int to LongWritable is ok, IntWritable to long is ok, but the other two combinations are not. giraph-core/src/main/java/org/apache/giraph/types/JavaWritableConverter.java <https://reviews.apache.org/r/12253/#comment46493> Just a suggestion: how about we call this something like WritableWrapper, with methods wrap() and unwrap()? giraph-hive/src/main/java/org/apache/giraph/hive/input/edge/TypedHiveToEdge.java <https://reviews.apache.org/r/12253/#comment46496> Default value is the empty string? giraph-hive/src/main/java/org/apache/giraph/hive/input/edge/TypedHiveToEdge.java <https://reviews.apache.org/r/12253/#comment46495> Newline after @Override. This will cause an AbstractMethodError otherwise. giraph-hive/src/main/java/org/apache/giraph/hive/input/vertex/TypedHiveToVertex.java <https://reviews.apache.org/r/12253/#comment46498> See above. giraph-hive/src/main/java/org/apache/giraph/hive/input/vertex/TypedHiveToVertex.java <https://reviews.apache.org/r/12253/#comment46497> See above. giraph-hive/src/main/java/org/apache/giraph/hive/output/TypedVertexToHive.java <https://reviews.apache.org/r/12253/#comment46499> See above. giraph-hive/src/main/java/org/apache/giraph/hive/output/TypedVertexToHive.java <https://reviews.apache.org/r/12253/#comment46500> See above. giraph-hive/src/main/java/org/apache/giraph/hive/output/TypedVertexToHive.java <https://reviews.apache.org/r/12253/#comment46501> See above. - Alessandro Presta On July 3, 2013, 4:43 p.m., Nitay Joffe wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/12253/ > ----------------------------------------------------------- > > (Updated July 3, 2013, 4:43 p.m.) > > > Review request for giraph. > > > Bugs: GIRAPH-705 > https://issues.apache.org/jira/browse/GIRAPH-705 > > > Repository: giraph-git > > > Description > ------- > > Type converters and giraph-hive cleanup > > > Diffs > ----- > > > giraph-core/src/main/java/org/apache/giraph/benchmark/PageRankBenchmark.java > d88ff0d8376357035c6944fb2dbdcb9c4614d3d3 > giraph-core/src/main/java/org/apache/giraph/jython/JythonUtils.java > f456aa8b6a665925ceabe8983df07239dfc171ad > > giraph-core/src/main/java/org/apache/giraph/types/BooleanToBooleanWritableConverter.java > PRE-CREATION > > giraph-core/src/main/java/org/apache/giraph/types/ByteToByteWritableConverter.java > PRE-CREATION > > giraph-core/src/main/java/org/apache/giraph/types/ByteToIntWritableConverter.java > PRE-CREATION > > giraph-core/src/main/java/org/apache/giraph/types/ByteToLongWritableConverter.java > PRE-CREATION > > giraph-core/src/main/java/org/apache/giraph/types/DoubleToDoubleWritableConverter.java > PRE-CREATION > > giraph-core/src/main/java/org/apache/giraph/types/DoubleToFloatWritableConverter.java > PRE-CREATION > > giraph-core/src/main/java/org/apache/giraph/types/FloatToDoubleWritableConverter.java > PRE-CREATION > > giraph-core/src/main/java/org/apache/giraph/types/FloatToFloatWritableConverter.java > PRE-CREATION > > giraph-core/src/main/java/org/apache/giraph/types/IntToByteWritableConverter.java > PRE-CREATION > > giraph-core/src/main/java/org/apache/giraph/types/IntToIntWritableConverter.java > PRE-CREATION > > giraph-core/src/main/java/org/apache/giraph/types/IntToLongWritableConverter.java > PRE-CREATION > > giraph-core/src/main/java/org/apache/giraph/types/JavaAndWritableClasses.java > PRE-CREATION > > giraph-core/src/main/java/org/apache/giraph/types/JavaWritableConverter.java > PRE-CREATION > > giraph-core/src/main/java/org/apache/giraph/types/LongToByteWritableConverter.java > PRE-CREATION > > giraph-core/src/main/java/org/apache/giraph/types/LongToIntWritableConverter.java > PRE-CREATION > > giraph-core/src/main/java/org/apache/giraph/types/LongToLongWritableConverter.java > PRE-CREATION > > giraph-core/src/main/java/org/apache/giraph/types/ShortToByteWritableConverter.java > PRE-CREATION > > giraph-core/src/main/java/org/apache/giraph/types/ShortToIntWritableConverter.java > PRE-CREATION > > giraph-core/src/main/java/org/apache/giraph/types/ShortToLongWritableConverter.java > PRE-CREATION > giraph-core/src/main/java/org/apache/giraph/types/TypeConverters.java > PRE-CREATION > giraph-core/src/main/java/org/apache/giraph/types/package-info.java > PRE-CREATION > giraph-core/src/main/java/org/apache/giraph/utils/ConfigurationUtils.java > f0a7e4f784f970250f6f515a261b19f15e18fbc7 > > giraph-core/src/main/java/org/apache/giraph/utils/DistributedCacheUtils.java > 6abe89b068e17823a3083061cb213c53c5b72f0d > giraph-core/src/test/java/org/apache/giraph/jython/TestJython.java > 245d342ddf903c83130e299a33d0bee74cfc6949 > giraph-hive/src/main/java/org/apache/giraph/hive/HiveGiraphRunner.java > 589fed6c746c8f988f36cc439e78bc389b4b6e40 > giraph-hive/src/main/java/org/apache/giraph/hive/common/HiveUtils.java > 11b060f603b32ead84799a818d4604b0b1ba14af > > giraph-hive/src/main/java/org/apache/giraph/hive/input/edge/SimpleHiveToEdge.java > 56b38f9d740aacdf72f1d710fbfa0c2de833936a > > giraph-hive/src/main/java/org/apache/giraph/hive/input/edge/TypedHiveToEdge.java > PRE-CREATION > > giraph-hive/src/main/java/org/apache/giraph/hive/input/vertex/TypedHiveToVertex.java > PRE-CREATION > > giraph-hive/src/main/java/org/apache/giraph/hive/output/AbstractVertexToHive.java > 477ce6e3da499d96e3dc50fd35edeae649c85192 > > giraph-hive/src/main/java/org/apache/giraph/hive/output/HiveVertexWriter.java > bb27f25c627f50c3b1bb48d027e42631f3b0855c > > giraph-hive/src/main/java/org/apache/giraph/hive/output/TypedVertexToHive.java > PRE-CREATION > giraph-hive/src/main/java/org/apache/giraph/hive/output/VertexToHive.java > f9537a779a47d7eff374dcf4db2557f3495f6969 > giraph-hive/src/main/java/org/apache/giraph/hive/types/HiveValueReader.java > PRE-CREATION > giraph-hive/src/main/java/org/apache/giraph/hive/types/HiveValueWriter.java > PRE-CREATION > > giraph-hive/src/main/java/org/apache/giraph/hive/types/HiveVertexIdReader.java > PRE-CREATION > > giraph-hive/src/main/java/org/apache/giraph/hive/types/HiveVertexIdWriter.java > PRE-CREATION > > giraph-hive/src/main/java/org/apache/giraph/hive/types/TypedValueReader.java > PRE-CREATION > > giraph-hive/src/main/java/org/apache/giraph/hive/types/TypedValueWriter.java > PRE-CREATION > > giraph-hive/src/main/java/org/apache/giraph/hive/types/TypedVertexIdReader.java > PRE-CREATION > > giraph-hive/src/main/java/org/apache/giraph/hive/types/TypedVertexIdWriter.java > PRE-CREATION > giraph-hive/src/main/java/org/apache/giraph/hive/types/package-info.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/12253/diff/ > > > Testing > ------- > > > Thanks, > > Nitay Joffe > >
