Replies: 12 comments 5 replies
-
I suppose |
Beta Was this translation helpful? Give feedback.
-
Branch under development here. I picked semantics consistent with |
Beta Was this translation helpful? Give feedback.
-
sorry for the slow response, sounds cool to me! |
Beta Was this translation helpful? Give feedback.
-
No sweat, I got distracted by something shiny and haven't worked on this for a while. |
Beta Was this translation helpful? Give feedback.
-
Makes sense to me! 🙂 For the map implementations, though, I think the method should be called Aside from that, a few small thoughts for your consideration:
But those are all minor things.
That makes sense! Maybe |
Beta Was this translation helpful? Give feedback.
-
Actually, wait, sorry, there's a bug: To fix this, we need to explicitly consider
(Disclaimer: not tested.) Instead of having tests for the internal |
Beta Was this translation helpful? Give feedback.
-
Amazing, thanks so much for these thoughtful reviews! I haven't had time to look at this for a few weeks but hopefully soon. |
Beta Was this translation helpful? Give feedback.
-
Hey I just realized PR #98 removed
Am I naively putting back functionality that is already known to be a bad idea? |
Beta Was this translation helpful? Give feedback.
-
Still hoping to get some time to look at this soon... |
Beta Was this translation helpful? Give feedback.
-
@prdoyle If you don't have time for this soon, would you be offended and/or disappointed if I were to take it over and update the PR per my comments above? (Note: I myself won't be able to get to this for at least the next few weeks, so "soon" is relative. No rush!) |
Beta Was this translation helpful? Give feedback.
-
Sorry for the delay here. I tried to work on this about a month and a half ago, but I found that, between Gradle settings and Java 9 modules and ParameterizedTests, it's become quite difficult to develop PCollections in Eclipse; I had to do a bunch of weird Eclipse setup, and even after that, there were still major problems (it couldn't resolve the imports in Benchmarks.java, and it couldn't handle the unit-tests that use ParameterizedTests). So, I assume that y'all are using a different IDE? It would be helpful if there were a bit of documentation somewhere about how to get set up — even if it's just a line saying "We use [IDE name], see [link to its documentation]" — unless there already is and I just haven't found it. (I notice that *.iml is mentioned in .gitignore, which suggests that at least one contributor used IntelliJ IDEA at some point; is that what you use today?) (Also, by the way, on the subject of new-user setup: This is on me, but I didn't realize until just now that I could address Gradle's complaint about |
Beta Was this translation helpful? Give feedback.
-
I use IntelliJ FWIW. |
Beta Was this translation helpful? Give feedback.
-
In my application, it would be nice to look up the position of a given key in an
OrderedPMap
; that is, how many elements are to the left of a given element. This should be straightforward to implement in log(n) time inKVTree
, and exposed inOrderedPMap
,OrderedPSet
,PSortedMap
, andPSortedSet
.If you're open to this idea, I could start working on an implementation.
A natural follow-up question would be: should we be able to fetch an elemenet given its index? I don't need that myself, but I could add it if we want it and it's straightforward enough. (Though I might need some help picking a method name; maybe
getAt
?)Beta Was this translation helpful? Give feedback.
All reactions