CollReduce and IKVReduce for nil?

Showing 1-5 of 5 messages
CollReduce and IKVReduce for nil? Wolodja Wentland 1/12/13 6:20 AM
Hi all,

I was wondering if there is some reason why CollFold and IKVReduce are not
extended to nil. A patch for this rots away in JIRA [0] and could probably be
merged as-is. Is this simply an oversight or a conscious design decision?

[0] http://dev.clojure.org/jira/browse/CLJ-1098
--
Wolodja <bab...@gmail.com>

4096R/CAF14EFC
081C B7CD FF04 2BA9 94EA  36B2 8B7F 7D30 CAF1 4EFC
Re: CollReduce and IKVReduce for nil? Andy Fingerhut 1/12/13 8:15 AM
I have no inside information on whether it is an oversight or a conscious design decision, but there are other possibilities besides those two.

The CLJ-1098 ticket was categorized as a minor enhancement when it was created.  Defects (i.e. bugs) are considered with higher priority.

Also Clojure 1.5.0 is nearing release [1], and pretty much any software must reduce its rate of changes near release so more testing can be done on a single version (or at most a few versions with minimal changes between them, if problems are found).

Thirdly, that ticket and its patch aren't exactly rotting, or at least no more so than any other unapplied patch for an open ticket.  That patch shows up on my list of prescreened patches [2].  Every patch for an open ticket gets re-evaluated every time a change is made to the latest version of Clojure to see whether it has gone "stale" (i.e. no longer applies cleanly due to a conflicting change).  If a patch goes stale, I either update it myself, or notify the patch authors that the patch needs updating.

If your concern is that it isn't being committed as fast as you would like it to, you have joined a new club.   Welcome. :-)  Until that ticket is considered and applied or rejected, you may create your own patched version of Clojure, and if you wish even distribute it under the Eclipse Public License, but those approaches might not suit your purposes.

[1] https://groups.google.com/forum/?fromgroups=#!topic/clojure/CAiajyDWJJg
[2] http://jafingerhut.github.com/clj-ticket-status/prescreened.txt
Re: CollReduce and IKVReduce for nil? Wolodja Wentland 1/14/13 4:24 AM
On Sat, Jan 12, 2013 at 08:15 -0800, Andy Fingerhut wrote:
> The CLJ-1098 ticket was categorized as a minor enhancement when it was
> created.  Defects (i.e. bugs) are considered with higher priority.

I can see that this categorisation caused this bug to be prioritised lower
than actual bugs that break documented (or previous) behaviour. I also think
that the categorisation is correct as the proposed change does, in fact,
constitute an enhancement of the /current/ behaviour.

The fact that this bug is an enhancement of the current behaviour as found
for IKVReduce in the current stable releases and for CollFold in the current
release candidates does, however, not mean that fixing it is not important, as
the current behaviour is just unintuitive and not in line with the behaviour
of nil in the context of other functions such as reduce.

I consider the "decision" to special case nil for reduce-kv and, soon, for
the reducers library a poor one. Fixing this oversight is very easy right now
and should be done as soon as possible and in particular before we find
libraries littered with (nil? coll).

> Also Clojure 1.5.0 is nearing release [1], and pretty much any software must
> reduce its rate of changes near release so more testing can be done on a
> single version (or at most a few versions with minimal changes between them,
> if problems are found).

Introducing reduce-kv was an important part of the 1.4 release and the
reducers library will be even more important for 1.5 and it should be a
priority that these two integrate well into the Clojure ecosystem and behave
in line with our expectations. The reducers library is even advertised as a
drop-in for existing codebases, but the implementation right now does not live
up to that and requires authors to special case nil, which is simply ugly.

> If your concern is that it isn't being committed as fast as you would like
> it to, you have joined a new club.   Welcome. :-)  Until that ticket is
> considered and applied or rejected, you may create your own patched version
> of Clojure, and if you wish even distribute it under the Eclipse Public
> License, but those approaches might not suit your purposes.

Well, I am sure that many people have their particular bug they would like to
see fixed, but I think it is a bad idea to /not/ fix this bug in 1.5 as the
reducers library violates expectations as it is now. One also does not /have/
to see the patch as an atomic one and I guess implementing only CollFold for
nil is an improvement over the status quo if doing so for IKVReduce is not
wanted (for whatever reason).

I was simply curious about why this has not been done in the first place and I
still do not see a good reason for doing so. I guess the main point I am
trying to make here is: Better fix it now before maintaining historic
behaviour/naming (such as the contains?-wart) becomes the only argument for
not applying the (very simple) patch.

If it is too late, then it is too late … but then I still don't understand
this design decision.
Re: CollReduce and IKVReduce for nil? Peter Taoussanis 1/14/13 8:53 AM
Just throwing in here that I was actually bit by IKVReduce just today. I swapped a reduce with a reduce-kv, assuming the two would function similarly. And, unfortunately, they did in the testing environment: it took a trip to production before a nil came up.

The current behavior is certainly unexpected, I'd say.
Re: CollReduce and IKVReduce for nil? Andy Fingerhut 1/25/13 11:59 AM
CLJ-1098 fix committed to Clojure master today as part of 1.5.0-RC3:

    http://build.clojure.org/job/clojure/changes

Andy