I agree with the principle of getting things to work first and then care about performance but here I think it would be a fundamentally flaw in the architecture that will necessarily cause very bad performance. One can get the test in the issue (which really if a request for enhancement) to pass without introducing these parts.
Reto On Thu, Jul 14, 2011 at 12:23 PM, Henry Story <[email protected]> wrote: > > On 14 Jul 2011, at 11:32, Reto Bachmann-Gmür wrote: > >> The issue I can agree with addressing is 603. While I think the naming >> is inappropriately dramatic as it would be better described as >> supporting a slightly different coding style, as I commented on the >> issue I could agree with a resolution. >> >> Not sure how to apply your patch, it doesn't seem to work with the >> regular path -p (see below). > > I don't know. That is what they recommend to do on > > http://www.apache.org/dev/git.html > > I agree that they should also give some info on how to apply the patch! > >> >> I think the changes in the patch pertinent to the issue are the >> changes to the bnode and the b_ methods, I don't see how other changes >> like >> >> def -->(sub: GraphNode): RichGraphNode = { >> - //RichGraphNode.this + sub >> + graph.addAll(sub.getGraph) >> -->(sub.getNode) >> } > > If you have want code like > > gr.bnode("danny") -- FOAF.knows --> ( gr.bnode("reto").a(FOAF.Person) ... ) > > to work then you need to allow the --> to take a GraphNode as object. It > should of course merge the > info. > > If you want to make it more efficient, you could open another issue to make > it more efficient. A simple > test of equality between the graphs would certainly help. > > > >> >> or >> >> + >> + /** >> + * Add one relation for each member of the iterable collection >> + */ >> + def -->>>(elems: Iterable[GraphNode]): RichGraphNode = { >> + for (res <- elems) -->(res) >> + RichGraphNode.this >> + } > > This is just so that you can allow relations to lists of GraphNodes. Sadly it > can't be the same as -->> as that clashes > with the other methods due to Type Erasure. > >> >> would relate to the issue. (And as a side-note, I don't think a >> potentially very expensive operation like copying around the whole >> context of a GraphNode should happen as a side effect of the --> >> method) > > I think it is better to file a new issue to make the code more efficient. > There are different ways to do that I am sure, so that could itself give rise > to a debate. No need to mix issues. Simplicity is the mantra now. Right ? :-) > > Henry > > > >> >> Reto >> >> >> Attempt to apply the patch: >> reto@reto-HPE-540ch:~/projects/apache/clerezza/trunk/parent$ wget >> https://issues.apache.org/jira/secure/attachment/12486181/0001-CLEREZZA-603-EzMGraph.bnode-now-returns-a-RichGraphN.patch >> --2011-07-14 11:14:53-- >> https://issues.apache.org/jira/secure/attachment/12486181/0001-CLEREZZA-603-EzMGraph.bnode-now-returns-a-RichGraphN.patch >> Resolving issues.apache.org... 140.211.11.121 >> Connecting to issues.apache.org|140.211.11.121|:443... connected. >> HTTP request sent, awaiting response... 200 OK >> Length: 10792 (11K) [text/plain] >> Saving to: `0001-CLEREZZA-603-EzMGraph.bnode-now-returns-a-RichGraphN.patch' >> >> 100%[===============================================================================================>] >> 10,792 --.-K/s in 0s >> >> 2011-07-14 11:14:54 (21.5 MB/s) - >> `0001-CLEREZZA-603-EzMGraph.bnode-now-returns-a-RichGraphN.patch' >> saved [10792/10792] >> >> reto@reto-HPE-540ch:~/projects/apache/clerezza/trunk/parent$ patch -p0 >> < 0001-CLEREZZA-603-EzMGraph.bnode-now-returns-a-RichGraphN.patch >> patching file >> b/parent/platform.security.foafssl/test/src/main/scala/org/apache/clerezza/foafssl/test/WebIDTester.scala >> Hunk #1 FAILED at 33. >> Hunk #2 FAILED at 122. >> Hunk #3 FAILED at 130. >> Hunk #4 FAILED at 148. >> Hunk #5 FAILED at 172. >> Hunk #6 FAILED at 199. >> Hunk #7 FAILED at 215. >> Hunk #8 FAILED at 223. >> Hunk #9 FAILED at 602. >> Hunk #10 FAILED at 709. >> Hunk #11 FAILED at 743. >> 11 out of 11 hunks FAILED -- saving rejects to file >> b/parent/platform.security.foafssl/test/src/main/scala/org/apache/clerezza/foafssl/test/WebIDTester.scala.rej >> patching file >> b/parent/rdf.scala.utils/src/main/scala/org/apache/clerezza/rdf/scala/utils/EzMGraph.scala >> Hunk #1 FAILED at 54. >> Hunk #2 FAILED at 72. >> 2 out of 2 hunks FAILED -- saving rejects to file >> b/parent/rdf.scala.utils/src/main/scala/org/apache/clerezza/rdf/scala/utils/EzMGraph.scala.rej >> patching file >> b/parent/rdf.scala.utils/src/main/scala/org/apache/clerezza/rdf/scala/utils/Preamble.scala >> Hunk #1 FAILED at 105. >> 1 out of 1 hunk FAILED -- saving rejects to file >> b/parent/rdf.scala.utils/src/main/scala/org/apache/clerezza/rdf/scala/utils/Preamble.scala.rej >> patching file >> b/parent/rdf.scala.utils/src/main/scala/org/apache/clerezza/rdf/scala/utils/RichGraphNode.scala >> Hunk #1 FAILED at 225. >> 1 out of 1 hunk FAILED -- saving rejects to file >> b/parent/rdf.scala.utils/src/main/scala/org/apache/clerezza/rdf/scala/utils/RichGraphNode.scala.rej >> patching file >> b/parent/rdf.scala.utils/src/test/scala/org/apache/clerezza/rdf/scala/utils/EzMGraphTest.scala >> Hunk #1 FAILED at 111. >> Hunk #2 FAILED at 120. >> Hunk #3 FAILED at 142. >> 3 out of 3 hunks FAILED -- saving rejects to file >> b/parent/rdf.scala.utils/src/test/scala/org/apache/clerezza/rdf/scala/utils/EzMGraphTest.scala.rej >> reto@reto-HPE-540ch:~/projects/apache/clerezza/trunk/parent$ svn status >> ? b >> ? 0001-CLEREZZA-603-EzMGraph.bnode-now-returns-a-RichGraphN.patch >> >> On Thu, Jul 14, 2011 at 11:05 AM, Henry Story <[email protected]> >> wrote: >>> Since there is a delay in the release I think it would be worth considering >>> a little >>> bit more closely the patches I submitted recently: >>> >>> - CLEREZZA-599 - EzMGraph imports too many implicits >>> - CLEREZZA-600 - remove need to define Preamble in code using >>> rdf.scala.utils >>> - CLEREZZA-603 - EzMGraph fails when working with more than one instance >>> >>> These are all very small patches I filed as requested. Each one is very >>> easy to understand and self >>> contained. CLEREZZA-603 is the one that makes the usefulness of the others >>> even clearer than what >>> they are by themselves. >>> >>> Henry Story >>> >>> Social Web Architect >>> http://bblfish.net/ >>> >>> > > Social Web Architect > http://bblfish.net/ > >
