On Thu, Aug 9, 2018 at 7:25 AM Rob Tompkins <chtom...@gmail.com> wrote:
> > > > On Aug 9, 2018, at 9:17 AM, Gilles <gil...@harfang.homelinux.org> wrote: > > > > On Thu, 9 Aug 2018 08:43:34 -0400, Rob Tompkins wrote: > >>> On Aug 9, 2018, at 8:38 AM, Gilles <gil...@harfang.homelinux.org> > wrote: > >>> > >>> On Thu, 9 Aug 2018 08:22:32 -0400, Rob Tompkins wrote: > >>>> Are we instead trying to remove the “extends” from all of those > classes? > >>> > >>> I'm not sure what you mean, sorry. > >>> > >>> "SamplerBase" was meant has a utility/shortcut/boiler-plate in order > >>> to copy the "rng" argument in one place, instead of having a field > >>> in each sampler class. It is purely internal to this library (in C++, > >>> inheritance would have been "private”). > >> > >> Are we trying to remove SamplerBase all together? > > > > That would break BC the same way. > > I wouldn't mind since the shortcut is not really significant. > > > > But my reasoning would be the same: correct usage does not refer > > to these methods so the fix could be done now, with minimal risk > > (due to low usage of a new component). > > > >>>> My thought was to override the method and javadoc locally (in the > >>>> actual subclass) so that we can give reasonable deprecation messages, > >>>> but that’s only a thought. I’m ok with whatever. > > > > It's overkill in this context. > > We shouldn't be rigid about the "no BC in minor release" rule; > > I’m a tad confused here. I thought that the whole point of everything that > we do in terms of the way we version and name packages > was specifically driven at the fact that we’re completely rigid about “no > BC breakage in minor releases.” > > I’ve not been around the project long enough to know this with certainty, > but that definitely is the vibe that I get. I would think that such RCs > would generally get -1’s from people. > > Generally I’m open to whatever. I’m just operating on what I perceive to > be precedent. > > @Gary - what’s the call here on the flexibility of BC incompatible changes > in a minor release? > BC is binary and we should not break stacks. You really don't want to create jar hell for our users. Think about deep transitive dependencies. An end use or developer is not going to care 2 cents about "correct vs incorrect API usage" when a component depends on this depends on that and so on, ends up breaking his or her whole build because a low level component has decided that breaking BC was OK. Gary > > enforcing it is worse for everybody: > > * bona fide users will be forced to modify their source and > > recompile in order to benefit from the performance boost > > introduced in version 1.1, and > > * bad usage (if it exists at all) could continue unsuspected. > > > >>> The message would aim at the wrong target: for developers of this > >>> library, there is no deprecation (the boiler-plate code is there > >>> to be used); for application developers, the class should not be > >>> there to be used (hence: package private). > >> > >> Then, I guess, we should simply state that we want to eventually > >> delete these methods and they should not be consumed. Yeah? > > > > No (cf. above); I propose to delete them *now*. > > > >> Like I > >> said…I’m up for whatever. I’m just trying to balance yours and Gary’s > >> points and find a good middle ground that everyone can live with. > > > > Which are the two sides for which you try to find a middle ground? > > At any rate it would not be "good" if it makes developers and users > > unhappy for a case that probably doesn't exist out there. > > Tools such as Clirr are a great help, but they shouldn't induce us > > to take the wrong course only to have the pleasure to contemplate a > > clean report. ;-) > > Thanks to Clirr (and Gary), we've become aware of a design issue. > > We can decide to resolve it as it was done in the past, with IMHO > > zero inconvenience (except for the Clirr "unclean" report). > > > > Regards, > > Gilles > > > >> -Rob > >> > >>> > >>> Gilles > >>> > >>>> -Rob > >>>> > >>>>> On Aug 9, 2018, at 7:02 AM, Gilles <gil...@harfang.homelinux.org> > wrote: > >>>>> > >>>>> Hi all. > >>>>> > >>>>> On Wed, 08 Aug 2018 19:59:24 +0200, Gilles wrote: > >>>>>> Hello Rob. > >>>>>> > >>>>>> On Wed, 8 Aug 2018 12:44:16 -0400, Rob Tompkins wrote: > >>>>>>> @Gilles - thoughts here?? Just kinda what I was thinking, but I’m > >>>>>>> only a +0 on this change. So, if you want to revert it before > going up > >>>>>>> with 1.1, that’s fine. > >>>>>> > >>>>>> I don't understand this change after what I answered to Gary's > strange > >>>>>> proposal. > >>>>>> The comment is *wrong*. As I said previously, the base class > provides > >>>>>> boiler-plate code; that is *not* deprecated. The issue is that > those > >>>>>> methods were not meant to be used further down the hierarchy (in > user's > >>>>>> subclasses). [And now, furthermore, they are not used anymore in > class > >>>>>> "PoissonSampler", following the change in RNG-50 (delegation to > other > >>>>>> classes).] > >>>>>> The sampler classes should have been made "final"; but now, this > change > >>>>>> would also be "breaking" (even though I doubt about legitimate > use-cases > >>>>>> for inherithing from the sampler implementations). > >>>>> > >>>>> Assuming it's unlikely that application developers would have > >>>>> * called protected methods of "SamplerBase",[1] > >>>>> * create subclasses of "SamplerBase",[1] > >>>>> * create subclasses of the sampler classes,[1] > >>>>> I suggest to > >>>>> * make "SamplerBase" package private > >>>>> * make the sampler clases "final". > >>>>> > >>>>> The above are the *less* intrusive fixes, as they would only > >>>>> potentially impact application developers by making them aware > >>>>> that they have made incorrect usage of an *inadvertently* > >>>>> public API. Correct usage will not be impacted even though > >>>>> the change is not BC.[2] > >>>>> > >>>>> Any objection to that fix?[3] > >>>>> > >>>>> Regards, > >>>>> Gilles > >>>>> > >>>>> [1] Rationale: (a) There is no reason to do that and > >>>>> (b) "Commons RNG" isn't much used at this point: > >>>>> > http://mvnrepository.com/artifact/org.apache.commons/commons-rng-sampling/usages > >>>>> [2] I recall that a similar situation (BC breaking in a minor > >>>>> release) occurred in CM, at a time where the number of > >>>>> potential users was much larger. > >>>>> [3] I'll add a prominent warning in the changelog to the effect > >>>>> that people wanting to continue with incorrect usage of the > >>>>> samplers should not upgrade. ;-) > >>>>> > >>>>>> > >>>>>> Please revert. > >>>>>> > >>>>>> Thanks, > >>>>>> Gilles > >>>>>> > >>>>>>> > >>>>>>> -Rob > >>>>>>> > >>>>>>>> On Aug 8, 2018, at 12:42 PM, chtom...@apache.org wrote: > >>>>>>>> > >>>>>>>> Repository: commons-rng > >>>>>>>> Updated Branches: > >>>>>>>> refs/heads/1.1 50b984b1d -> f8159f28a > >>>>>>>> > >>>>>>>> > >>>>>>>> Adding PoissonSampler deprecations. Use the correct faster public > methods > >>>>>>>> > >>>>>>>> > >>>>>>>> Project: http://git-wip-us.apache.org/repos/asf/commons-rng/repo > >>>>>>>> Commit: > http://git-wip-us.apache.org/repos/asf/commons-rng/commit/f8159f28 > >>>>>>>> Tree: > http://git-wip-us.apache.org/repos/asf/commons-rng/tree/f8159f28 > >>>>>>>> Diff: > http://git-wip-us.apache.org/repos/asf/commons-rng/diff/f8159f28 > >>>>>>>> > >>>>>>>> Branch: refs/heads/1.1 > >>>>>>>> Commit: f8159f28a52197d0e7b55e39b115702147cf57a0 > >>>>>>>> Parents: 50b984b > >>>>>>>> Author: Rob Tompkins <chtom...@gmail.com> > >>>>>>>> Authored: Wed Aug 8 12:42:45 2018 -0400 > >>>>>>>> Committer: Rob Tompkins <chtom...@gmail.com> > >>>>>>>> Committed: Wed Aug 8 12:42:45 2018 -0400 > >>>>>>>> > >>>>>>>> > ---------------------------------------------------------------------- > >>>>>>>> .../sampling/distribution/PoissonSampler.java | 42 > ++++++++++++++++++++ > >>>>>>>> 1 file changed, 42 insertions(+) > >>>>>>>> > ---------------------------------------------------------------------- > >>>>>>>> > >>>>>>>> > >>>>>>>> > http://git-wip-us.apache.org/repos/asf/commons-rng/blob/f8159f28/commons-rng-sampling/src/main/java/org/apache/commons/rng/sampling/distribution/PoissonSampler.java > >>>>>>>> > ---------------------------------------------------------------------- > >>>>>>>> diff --git > a/commons-rng-sampling/src/main/java/org/apache/commons/rng/sampling/distribution/PoissonSampler.java > b/commons-rng-sampling/src/main/java/org/apache/commons/rng/sampling/distribution/PoissonSampler.java > >>>>>>>> index d0733ba..29d0e4e 100644 > >>>>>>>> --- > a/commons-rng-sampling/src/main/java/org/apache/commons/rng/sampling/distribution/PoissonSampler.java > >>>>>>>> +++ > b/commons-rng-sampling/src/main/java/org/apache/commons/rng/sampling/distribution/PoissonSampler.java > >>>>>>>> @@ -67,6 +67,48 @@ public class PoissonSampler > >>>>>>>> return poissonSampler.sample(); > >>>>>>>> } > >>>>>>>> > >>>>>>>> + /** > >>>>>>>> + * @return a random value from a uniform distribution in the > >>>>>>>> + * interval {@code [0, 1)}. > >>>>>>>> + * @deprecated - one should be using the {@link > PoissonSampler#sample()} method, > >>>>>>>> + * as it is considerably faster. > >>>>>>>> + */ > >>>>>>>> + @Deprecated > >>>>>>>> + protected double nextDouble() { > >>>>>>>> + return super.nextDouble(); > >>>>>>>> + } > >>>>>>>> + > >>>>>>>> + /** > >>>>>>>> + * @return a random {@code int} value. > >>>>>>>> + * @deprecated - one should be using the {@link > PoissonSampler#sample()} method, > >>>>>>>> + * as it is considerably faster. > >>>>>>>> + */ > >>>>>>>> + @Deprecated > >>>>>>>> + protected int nextInt() { > >>>>>>>> + return super.nextInt(); > >>>>>>>> + } > >>>>>>>> + > >>>>>>>> + /** > >>>>>>>> + * @param max Upper bound (excluded). > >>>>>>>> + * @return a random {@code int} value in the interval {@code > [0, max)}. > >>>>>>>> + * @deprecated - one should be using the {@link > PoissonSampler#sample()} method, > >>>>>>>> + * * as it is considerably faster. > >>>>>>>> + */ > >>>>>>>> + @Deprecated > >>>>>>>> + protected int nextInt(int max) { > >>>>>>>> + return super.nextInt(max); > >>>>>>>> + } > >>>>>>>> + > >>>>>>>> + /** > >>>>>>>> + * @return a random {@code long} value. > >>>>>>>> + * @deprecated - one should be using the {@link > PoissonSampler#sample()} method, > >>>>>>>> + * * as it is considerably faster. > >>>>>>>> + */ > >>>>>>>> + @Deprecated > >>>>>>>> + protected long nextLong() { > >>>>>>>> + return super.nextLong(); > >>>>>>>> + } > >>>>>>>> + > >>>>>>>> /** {@inheritDoc} */ > >>>>>>>> @Override > >>>>>>>> public String toString() { > >>>>>>>> > > > > > > --------------------------------------------------------------------- > > To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org > > For additional commands, e-mail: dev-h...@commons.apache.org > > > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org > For additional commands, e-mail: dev-h...@commons.apache.org > >
