[histogram] Finalising version for submission

older
[all] Hidden visibility is on by...

Hans Dembinski

22 Oct 2018 22 Oct '18

1:25 p.m.

Hi all, I have a general question regarding the submission process on how to deal with extensions of the library. I am currently working on the points raised in the review. Clearly, the bugs need to be fixed and the documentation improved where it was lacking. So the procedure is clear in that regard, but the final version to be included in Boost will also have more features, the main ones are: - support for profiles and in general arbitrary accumulators as bin "counter" types - in particular any Boost.Accumulator - allow arbitrary metadata (or no metadata at all) to be associated with the builtin axes types, instead of just supporting a string label (the metadata type will be a template parameter) - use axis types through a traits-interface to make adding custom axis types really easy (a minimal axis only needs to implement `int operator()(something) const` and `unsigned size() const`, everything else is optional) - use storage types through a traits-interface to allow arbitrary STL and Boost containers as storage backends - add a support library for typical calculations done with histograms, like reducing a high-dimensional histogram to a lower-dimensional histogram, shrink an axis, etc - growing axes that increase the number of bins while the histogram is filled I started working on these features and switching to C++14 would simplify the implementation a lot, thanks to generic lambdas and return type deduction. So I would like to raise the requirement for the final library from C++11 to C++14. Since these are substantial changes, is another formal review of the final version required? In any case, I will request informal feedback on the final version on the list before histogram is released with Boost for the first time. Best regards, Hans

Show replies by date

Alexander Grund

22 Oct 22 Oct

1:45 p.m.

...

I started working on these features and switching to C++14 would simplify the implementation a lot, thanks to generic lambdas and return type deduction. So I would like to raise the requirement for the final library from C++11 to C++14.

Disclaimer: I was NOT involved in the review, so the following is merely my opinion. There was currently discussion about "switching to C++11" on the list and an often brought argument was "the library maintainer is free to raise the requirement at any time" and "there are already Boost.Libs requiring C++11/14/17". So I assume this change won't be a problem especially as it is C++11->14 (minor change) opposed to C++98->11 (major change) For the rest: As long as you *extend* the interface and not *change* it, there should not be a problem. Additional (reasonable) features to accepted libraries are allowed. Alex

Mateusz Loskot

1:55 p.m.

On Mon, 22 Oct 2018 at 15:45, Alexander Grund via Boost wrote:

...

...
I started working on these features and switching to C++14 would simplify the implementation a lot, thanks to generic lambdas and return type deduction. So I would like to raise the requirement for the final library from C++11 to C++14.

There was currently discussion about "switching to C++11" on the list and an often brought argument was "the library maintainer is free to raise the requirement at any time" and "there are already Boost.Libs requiring C++11/14/17".

Folks, If this is indeed a generally accepted practice, can we have it documented on the website, please? That seems like quite an important guideline, doesn't it. Best regards, -- Mateusz Loskot, http://mateusz.loskot.net

Peter Dimov

2:29 p.m.

Mateusz Loskot wrote:

...

On Mon, 22 Oct 2018 at 15:45, Alexander Grund via Boost wrote:

...
...
So I would like to raise the requirement for the final library from C++11 to C++14.

There was currently discussion about "switching to C++11" on the list and an often brought argument was "the library maintainer is free to raise the requirement at any time" and "there are already Boost.Libs requiring C++11/14/17".

Folks,

If this is indeed a generally accepted practice, can we have it documented on the website, please?

It's not. "At any time" is not generally accepted to include the time between the review and the appearance of the library in Boost, _unless the review manager and the people who gave positive reviews don't object_. Something close to what was accepted is generally expected to appear in at least one Boost release. Bait and switch would not be fair to the reviewers and does not strike me as a particularly defensible practice.

Mateusz Loskot

3:21 p.m.

On Mon, 22 Oct 2018 at 16:29, Peter Dimov via Boost wrote:

...

Mateusz Loskot wrote:

...
On Mon, 22 Oct 2018 at 15:45, Alexander Grund via Boost wrote:

...
...
So I would like to raise the requirement for the final library from C++11 to C++14.

There was currently discussion about "switching to C++11" on the list and an often brought argument was "the library maintainer is free to raise the requirement at any time" and "there are already Boost.Libs requiring C++11/14/17".

Folks,

If this is indeed a generally accepted practice, can we have it documented on the website, please?

It's not.

"At any time" is not generally accepted to include the time between the review and the appearance of the library in Boost, _unless the review manager and the people who gave positive reviews don't object_.

AFAICT, post-review role of a review manager is not documented anywhere. Since I happened to be RM once or twice, I'm interested in any formal procedures I might have be missing.

...

Something close to what was accepted is generally expected to appear in at least one Boost release. Bait and switch would not be fair to the reviewers and does not strike me as a particularly defensible practice.

Peter, This makes sense, of course, and a typical sequence of events is roughly: 1. Review 2. Accepted 3. Released as accepted 4. Development continues, and authors "are free to change your library in any way you wish (...) are also encouraged to get feedback from the boost community before making substantial changes to the interface of an accepted library." 5. Released as developed 6. Repeat 4-5 I learn as I go, I confess. Hans, In the spirit to "be fair to the reviewers" indeed, I would encourage you to first release C++11 version of Histogram, stick it to `master` and then move on with C++14 in `develop`. Perhaps you could even manage to get the first release ready for... this Wednesday :-) that is, when Boost 1.69.0 closed for new libraries https://www.boost.org/development/index.html I feel bad about being late with the announcement of the review results - I have missed the Boost 1.69 dates myself completely. Best regards, -- Mateusz Loskot, http://mateusz.loskot.net

Hans Dembinski

4:05 p.m.

...

On 22. Oct 2018, at 17:21, Mateusz Loskot via Boost wrote:

In the spirit to "be fair to the reviewers" indeed, I would encourage you to first release C++11 version of Histogram, stick it to `master` and then move on with C++14 in `develop`. Perhaps you could even manage to get the first release ready for... this Wednesday :-) that is, when Boost 1.69.0 closed for new libraries https://www.boost.org/development/index.html

I have not fixed all the bugs yet found during the review, so histogram will not make it by Wednesday ;).

Hans Dembinski

3:57 p.m.

...

On 22. Oct 2018, at 16:29, Peter Dimov via Boost wrote:

...
If this is indeed a generally accepted practice, can we have it documented on the website, please?

It's not.

"At any time" is not generally accepted to include the time between the review and the appearance of the library in Boost, _unless the review manager and the people who gave positive reviews don't object_.

The switch from C++11 to C++14 would help me to make the library implementation simpler, it does not affect the user side. I originally chose C++11 to support older compilers, accepting the increased code complexity as a trade-off. But writing out-of-place structs in the detail namespace to emulate `if constexpr` is not really nice and now that I want to make the library even more flexible, I need to do something like `if constexpr` even more often. With C++14 I can at least emulate `if constexpr` with in-place lambdas, which makes the implementation more readable and compact. If the reviewers feel strongly about it, I will stick to C++11, but it was my impression that there was no strong preference for C++11. I was even asked the opposite by several people, e.g. by degski, why I was sticking to C++11 for a new library and whether the library could gain from C++14 or even C++17 features.

...

Something close to what was accepted is generally expected to appear in at least one Boost release. Bait and switch would not be fair to the reviewers and does not strike me as a particularly defensible practice.

I don't see myself "baiting and switching". I am implementing features that were requested during the review. I want to work through the wish-list before the library is released for the first time, because now I can still modify the interface if necessary. Once histogram is released with Boost, the interface is much harder to change. Best regards, Hans

Peter Dimov

4:05 p.m.

Hans Dembinski wrote:

...

The switch from C++11 to C++14 would help me to make the library implementation simpler, it does not affect the user side. I originally chose C++11 to support older compilers, accepting the increased code complexity as a trade-off. But writing out-of-place structs in the detail namespace to emulate `if constexpr` is not really nice and now that I want to make the library even more flexible, I need to do something like `if constexpr` even more often. With C++14 I can at least emulate `if constexpr` with in-place lambdas, which makes the implementation more readable and compact.

As I said, it's not a problem if the review manager and the reviewers do not object.

...

I don't see myself "baiting and switching". I am implementing features that were requested during the review.

The reference to bait and switch was a general response about the procedure and the claim that the author - in general - can do whatever he likes. It wasn't about this specific case, and I in no way imply that you are employing bait and switch of any kind. In this specific case, if you're going to transition to C++14 anyway, it would even be better for your users if your first release is C++14.

Mateusz Loskot

4:24 p.m.

On Mon, 22 Oct 2018 at 18:06, Peter Dimov via Boost wrote:

...

Hans Dembinski wrote:

...
The switch from C++11 to C++14 (...)

As I said, it's not a problem if the review manager and the reviewers do not object. (...) In this specific case, if you're going to transition to C++14 anyway, it would even be better for your users if your first release is C++14.

I don't object switch to C++14, on the contrary. I am trying to step into boots of reviewers though, hence my encouregement to stick to the original plan. And, to keep my rear aspect of personality safer ;) in case some reviewers object the switch - AFAIU, I will be the one to solve the dispute, and do it without any tool like Boost policy or guideline at my disposal. I do acknowledge that from maintenance point, it is better for Hans to release Histogram as C++14 library. I agree, it would be better for users too. Hans, let's get it done this way: - announce your plan to reviewers and request for feedback - give some reviewers a week to expresses any objections and move forward. I wouldn't expect withdrawal of ACCEPT vote from any of our reviewers, but who knows.... Best regards, -- Mateusz Loskot, http://mateusz.loskot.net

Hans Dembinski

4:26 p.m.

...

On 22. Oct 2018, at 18:05, Peter Dimov via Boost wrote:

Hans Dembinski wrote:

...
The switch from C++11 to C++14 would help me to make the library implementation simpler, it does not affect the user side. I originally chose C++11 to support older compilers, accepting the increased code complexity as a trade-off. But writing out-of-place structs in the detail namespace to emulate `if constexpr` is not really nice and now that I want to make the library even more flexible, I need to do something like `if constexpr` even more often. With C++14 I can at least emulate `if constexpr` with in-place lambdas, which makes the implementation more readable and compact.

As I said, it's not a problem if the review manager and the reviewers do not object.

Ok, I am going to write the reviewers about the switch from C++11 to C++14.

...

...
I don't see myself "baiting and switching". I am implementing features that were requested during the review.

The reference to bait and switch was a general response about the procedure and the claim that the author - in general - can do whatever he likes. It wasn't about this specific case, and I in no way imply that you are employing bait and switch of any kind.

Ok, thanks for clarifying. I assumed that it was a general comment, but preemptively gave my view of the situation in case my assumption was incorrect. ;)

Paul A. Bristow

3:39 p.m.

...

-----Original Message----- From: Boost [mailto:boost-bounces@lists.boost.org] On Behalf Of Hans Dembinski via Boost Sent: 22 October 2018 14:26 To: Boost Cc: Hans Dembinski Subject: [boost] [histogram] Finalising version for submission

...

I started working on these features and switching to C++14 would simplify the implementation a lot, thanks to generic lambdas and return type deduction. So I would like to raise the requirement for the final library from C++11 to C++14.

My view is that this is definitely the authors prerogative. But running it by the reviewers could not be a bad idea to quiet any muttering.

...

Since these are substantial changes, is another formal review of the final version required? In any case, I will request informal feedback on the final version on the list before histogram is released with Boost for the first time.

Sounds fine to me. Paul --- Paul A. Bristow Prizet Farmhouse Kendal UK LA8 8AB +44 (0) 1539 561830

2211

Age (days ago)

2211

Last active (days ago)

List overview

Download

10 comments

5 participants

participants (5)

Alexander Grund
Hans Dembinski
Mateusz Loskot
Paul A. Bristow
Peter Dimov