gwtd3 / gwt-d3

A GWT wrapper library around the d3.js library
Other
131 stars 53 forks source link

Force Node implementation #104

Closed vasvir closed 9 years ago

vasvir commented 9 years ago

Hi,

After the force implementation we noticed the following problem.

The Node class we are using is com.github.gwtd3.api.layout.Node extends JavascriptObject and corresponds to Tree, cluster and partitions. The doc is here https://github.com/mbostock/d3/wiki/Tree-Layout#nodes

The Force layout on the other hand has a different definition at https://github.com/mbostock/d3/wiki/Force-Layout#nodes. The Force Node in my fork extends com.github.gwtd3.api.layout.Node but I am wandering what is the correct approach given that the Link getSource() and getTarget() depends on Node definition.

So I am thinking 1) Create an empty class Node extends JavascriptObject {} in com.github.gwtd3.api.layout - required by Link compatibility 2) Move the current (Tree) Node implementation inside Tree (Tree.Node) Layout, or rename it to TreeNode, and extend the empty com.github.gwtd3.api.layout.Node 3) Implement the Force Node inside the Force (Force.Node), or rename it to ForceNode, and extend the empty com.github.gwtd3.api.layout.Node

I am willing to do the work and submit pull request if you tell me that you agree with this plan and which way you prefer it 1) TreeNode, ForceNode 2) Tree.Node, Force.Node

I am favoring 2) because D3 calls them Node not TreeNode or ForceNode so we have to play trick with packaging or internal classes to get the same names for multiple classes.

Vassilis Virvilis

anthonime commented 9 years ago

Hi,

I would also go for the solution #2.

Anthony

2014-10-10 9:55 GMT+02:00 Vassilis Virvilis notifications@github.com:

Hi,

After the force implementation we noticed the following problem.

The Node class we are using is com.github.gwtd3.api.layout.Node extends JavascriptObject and corresponds to Tree, cluster and partitions. The doc is here https://github.com/mbostock/d3/wiki/Tree-Layout#nodes

The Force layout on the other hand has a different definition at https://github.com/mbostock/d3/wiki/Force-Layout#nodes. The Force Node in my fork extends com.github.gwtd3.api.layout.Node but I aw wandering what is the correct approach given that the Link getSource() and getTarget() depends on Node definition.

So I am thinking 1) Create an empty class Node extends JavascriptObject {} in com.github.gwtd3.api.layout - required by Link compatibility 2) Move the current (Tree) Node implementation inside Tree (Tree.Node) Layout, or rename it to TreeNode, and extend the empty com.github.gwtd3.api.layout.Node 3) Implement the Force Node inside the Force (Force.Node), or rename it to ForceNode, and extend the empty com.github.gwtd3.api.layout.Node

I am willing to do the work and submit pull request if you tell me that you agree with this plan and which way you prefer it 1) TreeNode, ForceNode 2) Tree.Node, Force.Node

I am favoring 2) because D3 calls them Node not TreeNode or ForceNode so we have to play trick with packaging or internal classes to get the same names for multiple classes.

Vassilis Virvilis

— Reply to this email directly or view it on GitHub https://github.com/gwtd3/gwt-d3/issues/104.

Anthoni Schiochet +33 (0)6 51 81 11 87

anthonime commented 9 years ago

Hi Vassilis,

I accepted your merged request a bit too early... I didn't checked before merging, but unfortunately, the CI failed. There is a compilation error as stated in the trace:

[ERROR] COMPILATION ERROR : 
[INFO] -------------------------------------------------------------
[ERROR] /scratch/jenkins/workspace/CI of gwt-d3/gwt-d3-demo/src/main/java/com/github/gwtd3/demo/client/democases/layout/ClusterDendogram.java:[118,33] error: method nodes in class HierarchicalLayout cannot be applied to given types;
[ERROR]   required: com.github.gwtd3.api.layout.HierarchicalLayout.Node
  found: com.github.gwtd3.api.layout.Node
  reason: actual argument com.github.gwtd3.api.layout.Node cannot be converted to com.github.gwtd3.api.layout.HierarchicalLayout.Node by method invocation conversion
/scratch/jenkins/workspace/CI of gwt-d3/gwt-d3-demo/src/main/java/com/github/gwtd3/demo/client/democases/layout/ClusterDendogram.java:[119,33] error: method links in class HierarchicalLayout cannot be applied to given types;
[ERROR]   required: Array<com.github.gwtd3.api.layout.HierarchicalLayout.Node>
  found: Array<com.github.gwtd3.api.layout.Node>
  reason: actual argument Array<com.github.gwtd3.api.layout.Node> cannot be converted to Array<com.github.gwtd3.api.layout.HierarchicalLayout.Node> by method invocation conversion
/scratch/jenkins/workspace/CI of gwt-d3/gwt-d3-demo/src/main/java/com/github/gwtd3/demo/client/democases/layout/ClusterDendogram.java:[154,28] error: cannot find symbol
[ERROR]   symbol:   method children()
  location: class Node
/scratch/jenkins/workspace/CI of gwt-d3/gwt-d3-demo/src/main/java/com/github/gwtd3/demo/client/democases/layout/ClusterDendogram.java:[146,28] error: cannot find symbol
[ERROR]   symbol:   method children()
  location: class Node
/scratch/jenkins/workspace/CI of gwt-d3/gwt-d3-demo/src/main/java/com/github/gwtd3/demo/client/democases/TreeDemo.java:[137,10] error: cannot find symbol
[ERROR]   symbol:   method children()
  location: variable root of type TreeDemoNode
/scratch/jenkins/workspace/CI of gwt-d3/gwt-d3-demo/src/main/java/com/github/gwtd3/demo/client/democases/TreeDemo.java:[138,7] error: cannot find symbol
[ERROR]   symbol:   method children()
  location: variable root of type TreeDemoNode
/scratch/jenkins/workspace/CI of gwt-d3/gwt-d3-demo/src/main/java/com/github/gwtd3/demo/client/democases/TreeDemo.java:[150,26] error: method nodes in class HierarchicalLayout cannot be applied to given types;
[ERROR]   required: Node
  found: TreeDemoNode
  reason: actual argument TreeDemoNode cannot be converted to Node by method invocation conversion
/scratch/jenkins/workspace/CI of gwt-d3/gwt-d3-demo/src/main/java/com/github/gwtd3/demo/client/democases/TreeDemo.java:[151,26] error: method links in class HierarchicalLayout cannot be applied to given types;
[ERROR]   required: Array<com.github.gwtd3.api.layout.HierarchicalLayout.Node>
  found: Array<com.github.gwtd3.api.layout.Node>
  reason: actual argument Array<com.github.gwtd3.api.layout.Node> cannot be converted to Array<com.github.gwtd3.api.layout.HierarchicalLayout.Node> by method invocation conversion
/scratch/jenkins/workspace/CI of gwt-d3/gwt-d3-demo/src/main/java/com/github/gwtd3/demo/client/democases/TreeDemo.java:[159,28] error: cannot find symbol
[ERROR]   symbol:   method depth()
  location: variable datum of type TreeDemoNode
/scratch/jenkins/workspace/CI of gwt-d3/gwt-d3-demo/src/main/java/com/github/gwtd3/demo/client/democases/TreeDemo.java:[278,31] error: cannot find symbol
[ERROR]   symbol:   method children()
  location: variable datum of type TreeDemoNode
/scratch/jenkins/workspace/CI of gwt-d3/gwt-d3-demo/src/main/java/com/github/gwtd3/demo/client/democases/TreeDemo.java:[293,11] error: cannot find symbol
[ERROR]   symbol:   method children()
  location: variable node of type TreeDemoNode
/scratch/jenkins/workspace/CI of gwt-d3/gwt-d3-demo/src/main/java/com/github/gwtd3/demo/client/democases/TreeDemo.java:[294,34] error: cannot find symbol
[INFO] 12 errors 

Could you please fix it ? Thank you

vasvir commented 9 years ago

Hi

Sorry about that.

I will fix it first thing on Monday morning. In the mean time you can revert if you want and I will resubmit anyway.

How can I reproduce the failing? Is there any maven target that generates the error.

Looks like some import need readjustment...

 Vassilis

On Sun, Oct 19, 2014 at 8:45 PM, anthonime notifications@github.com wrote:

Hi Vassilis,

I accepted your merged request a bit too early... I didn't checked before merging, but unfortunately, the CI failed. There is a compilation error as stated in the trace:

[ERROR] COMPILATION ERROR : [INFO] ------------------------------------------------------------- [ERROR] /scratch/jenkins/workspace/CI of gwt-d3/gwt-d3-demo/src/main/java/com/github/gwtd3/demo/client/democases/layout/ClusterDendogram.java:[118,33] error: method nodes in class HierarchicalLayout cannot be applied to given types; [ERROR] required: com.github.gwtd3.api.layout.HierarchicalLayout.Node found: com.github.gwtd3.api.layout.Node reason: actual argument com.github.gwtd3.api.layout.Node cannot be converted to com.github.gwtd3.api.layout.HierarchicalLayout.Node by method invocation conversion /scratch/jenkins/workspace/CI of gwt-d3/gwt-d3-demo/src/main/java/com/github/gwtd3/demo/client/democases/layout/ClusterDendogram.java:[119,33] error: method links in class HierarchicalLayout cannot be applied to given types; [ERROR] required: Array found: Array reason: actual argument Array cannot be converted to Array by method invocation conversion /scratch/jenkins/workspace/CI of gwt-d3/gwt-d3-demo/src/main/java/com/github/gwtd3/demo/client/democases/layout/ClusterDendogram.java:[154,28] error: cannot find symbol [ERROR] symbol: method children() location: class Node /scratch/jenkins/workspace/CI of gwt-d3/gwt-d3-demo/src/main/java/com/github/gwtd3/demo/client/democases/layout/ClusterDendogram.java:[146,28] error: cannot find symbol [ERROR] symbol: method children() location: class Node /scratch/jenkins/workspace/CI of gwt-d3/gwt-d3-demo/src/main/java/com/github/gwtd3/demo/client/democases/TreeDemo.java:[137,10] error: cannot find symbol [ERROR] symbol: method children() location: variable root of type TreeDemoNode /scratch/jenkins/workspace/CI of gwt-d3/gwt-d3-demo/src/main/java/com/github/gwtd3/demo/client/democases/TreeDemo.java:[138,7] error: cannot find symbol [ERROR] symbol: method children() location: variable root of type TreeDemoNode /scratch/jenkins/workspace/CI of gwt-d3/gwt-d3-demo/src/main/java/com/github/gwtd3/demo/client/democases/TreeDemo.java:[150,26] error: method nodes in class HierarchicalLayout cannot be applied to given types; [ERROR] required: Node found: TreeDemoNode reason: actual argument TreeDemoNode cannot be converted to Node by method invocation conversion /scratch/jenkins/workspace/CI of gwt-d3/gwt-d3-demo/src/main/java/com/github/gwtd3/demo/client/democases/TreeDemo.java:[151,26] error: method links in class HierarchicalLayout cannot be applied to given types; [ERROR] required: Array found: Array reason: actual argument Array cannot be converted to Array by method invocation conversion /scratch/jenkins/workspace/CI of gwt-d3/gwt-d3-demo/src/main/java/com/github/gwtd3/demo/client/democases/TreeDemo.java:[159,28] error: cannot find symbol [ERROR] symbol: method depth() location: variable datum of type TreeDemoNode /scratch/jenkins/workspace/CI of gwt-d3/gwt-d3-demo/src/main/java/com/github/gwtd3/demo/client/democases/TreeDemo.java:[278,31] error: cannot find symbol [ERROR] symbol: method children() location: variable datum of type TreeDemoNode /scratch/jenkins/workspace/CI of gwt-d3/gwt-d3-demo/src/main/java/com/github/gwtd3/demo/client/democases/TreeDemo.java:[293,11] error: cannot find symbol [ERROR] symbol: method children() location: variable node of type TreeDemoNode /scratch/jenkins/workspace/CI of gwt-d3/gwt-d3-demo/src/main/java/com/github/gwtd3/demo/client/democases/TreeDemo.java:[294,34] error: cannot find symbol [INFO] 12 errors

Could you please fix it ? Thank you

— Reply to this email directly or view it on GitHub https://github.com/gwtd3/gwt-d3/issues/104#issuecomment-59657755.

Vassilis Virvilis

anthonime commented 9 years ago

I presently do not have a workstation to revert the change... No problem, if you fix it tomorrow.

To reproduce the error, just mvn clean install the whole project (not only the api module, but also the demo module).

vasvir commented 9 years ago

Ok I issued a pull request #108 with the fixes yesterday. Let me know if you notice any problem.