mahout-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Deneche A. Hakim (JIRA)" <j...@apache.org>
Subject [jira] Issue Comment Edited: (MAHOUT-184) Code tweaks for .df.* code
Date Sat, 03 Oct 2009 06:33:23 GMT

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

Deneche A. Hakim edited comment on MAHOUT-184 at 10/2/09 11:32 PM:
-------------------------------------------------------------------

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}


      was (Author: adeneche):
    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.


Mime
View raw message