Hi Everyone,
Please, welcome the Histogram library to the Boost collection!
Thank you Hans for submitting a high-quality library for review.
Thank you to the reviewers as well as those who just exchanged
their comments on numerous aspects of the library
and participated in the fruitful discussions.
Among reviewers, we can recognise veteran Boost contributors and
authors as well as experts in the fields of statistics, raster analysis,
including authors of histogram-oriented software packages.
----------------------------------------
Official reviews and votes:
----------------------------------------
1. Klemens David Morgenstern - ACCEPT
2. Jim Pivarski (off-list) - ACCEPT
3. Bjorn Reese - ACCEPT conditionally
4. Steven Watanabe - ACCEPT conditionally
5. Alex Hagen-Zanker - ACCEPT
Non-review participations:
1. Andrea Bocci (off-list)
2. degski
3. Glen Fernandes (off-list)
4. Gavin Lambert
5. Seth
6. Mateusz Loskot
----------------------------------------
Acceptance conditions
----------------------------------------
Two reviews stated conditions which are important fulfilled:
Bjorn Reese
- Reference documentation is finalized
Steven Watanabe
- Various exception safety and memory issues must be fixed.
- The reference documentation must be completed. Specifically,
various preconditions, requirements,
and assumptions must be made explicit.
I believe those are not controversial at all and
Hans is also interested in addressing those requests.
----------------------------------------
Concerns and comments
----------------------------------------
Jim Pivarski mentioned importance of profile plots as fundamental enough
that they should be part of the feature set.
In short term, the profile plots can be implemented as a new storage type,
using the extension capabilities.
Similarly, some concurrent applications would need atomic-valued bins,
but these, too, can be storage types.
Bjorn Reese pointed out some aspects of the design that makes the library
not work very well with STL algorithms. Bjorn strongly recommended that
the default storage policy is changed to remove some of those limitations.
Steven Watanabe provided Hans with large number of very detailed comments
and fixes (eg. do not call destructor which has been fixed),
suggestions how to improve run-fail tests and serialization tests.
Alex Hagen-Zanker commented that range-based for-loops support could
be improved,
suggested to provide separate ranges or views for certain histogram
access scenarios.
Another important theme of Alex' review was Histogram applicability for
moving window type analysis. Then, Alex requested clarification on choosing
variance based on the Poisson distribution instead of the binomial distribution.
degski suggested to look for some "low hanging fruits" in C++14 and/or C++17,
the library could potentially benefit from, pushing the 'modern C++'
trait of the library even further.
Gavin Lambert suggested to allow use of quantity types like Boost.Units.
Glen Fernandes mentioned he would raise allocator model support issues
or relaxing the requirement that the pointer must be a raw pointer of
type value_type*. Glen mentioned he may even be available to help.
Hans has already addressed many of the suggestions and issues or planned
related features for future releases of the library.
Issues resulting from the feedback received during the review can be
tracked and further discussed on GitHub, labelled with boost-review:
https://github.com/HDembinski/histogram/labels/boost-review
----------------------------------------
The Upsides
----------------------------------------
There were lots of great things said about the Histogram library:
- design: excellent, clean, simple and extensible in principle
- useful and well-designed features
- geared towards readability and performance
- very good documentation
----------------------------------------
Final Thoughts
----------------------------------------
Reviewers are worth their weight in gold nowadays.
Once again, I'd like to thank all participants for great
amount of quality feedback that already led to important
improvements in the Histogram library in very timely manner
- thank you Hans!
The Histogram library is a valuable addition to Boost.
Best regards,
--
Mateusz Loskot
Review Manager for the proposed Boost.Histogram
