[ 
https://issues.apache.org/jira/browse/MAHOUT-184?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12761819#action_12761819
 ] 

Deneche A. Hakim commented on MAHOUT-184:
-----------------------------------------

I'm having trouble applying the patch, got a lot of: "Hunk #1 FAILED at XX."
I inspected the patch without applying it and it look fine, although I have two 
comments:

* comment 1:
in core/main/.../df/node/Node.java you added the following comment:
{code}
   private int node2Type() {
+    // TODO shouldn't subclasses override this method then??
{/code}

the problem is that node2type() is used when writing a Node to actually store 
its class in the form of an Integer. The static method Node.read(DataInput in) 
is used to read the node delegating the reading to the correct implementation. 
Overriding node2Type() without modifying Node.read(DataInput in) is a certain 
way to have headaches !!!
A better solution is to store the class name and use class loaders, but for now 
I don't really need it.

* comment 2:
in core/src/test/java/org/apache/mahout/df/data/DataLoaderTest.java you made 
the following change:

{code}
+  protected static void checkCategorical(double[][] source, List<Integer> 
missings,
       Data loaded, int attr, int aId, double oValue, double nValue) {
     int lind = 0;
 
@@ -286,7 +277,7 @@
         continue;
 
       if (source[index][attr] == oValue) {
-        assertTrue(nValue == loaded.get(lind).get(aId));
+        assertEquals(nValue, loaded.get(lind).get(aId), 0.0);
{/code}

I think you meant :

{code}
+        assertEquals(nValue, loaded.get(lind).get(aId)); 
{/code}


> Code tweaks for .df.* code
> --------------------------
>
>                 Key: MAHOUT-184
>                 URL: https://issues.apache.org/jira/browse/MAHOUT-184
>             Project: Mahout
>          Issue Type: Improvement
>            Reporter: Sean Owen
>            Assignee: Sean Owen
>            Priority: Minor
>             Fix For: 0.2
>
>         Attachments: Tweaks_to__df__.patch
>
>
> This follows on my last email to the mailing list, and code inspection. It's 
> big enough I made a patch. No surprises I hope given the consensus on code 
> style and practice. Might be some good takeaways in here, or points for 
> further discussion.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

Reply via email to