[mlpack-git] [mlpack] Remove ToString() entirely (#487)

Ryan Curtin notifications at github.com
Tue Dec 1 18:14:24 EST 2015 

I'll leave this pull request open for a week, for the sake of discussion, since it's a major change

In the past, the great idea was come upon that a `ToString()` method should be implemented in every class, then some glue logic could allow `operator<<()` for any arbitrary mlpack object.  This was done with template metaprogramming (see `sfinae_utility.hpp` by Trironk, it is quite cool) and worked fine.

However, as time has gone on, it's unclear to me whether or not this feature is actually useful, and personally I am of the opinion that it should be removed, so I made a few commits removing all the `ToString()` functions and related functionality and made this pull request.  Here are my arguments for why it isn't very useful:

 * It's not always clear how much should be printed in a `ToString()` call.  Should we print all of a binary space tree?  Only the first few levels?  How much information does the user want?  In many cases, the amount of information that *could* be printed is huge, so when a user sends an object to an ostream, they get reams and reams of output.

 * It's extra overhead for new contributors and also for maintenance, if their classes also have to have a `ToString()` method.  And usually when these are written, it seems like simple copypasta and not much thought is given into precisely what to display (see the previous point).

 * It's not actually all that useful for debugging; often when debugging, I find myself ignoring the `ToString()` function and instead only printing the member of the object that I am interested in.

So, those are the primary reasons I think that we should remove---or at least rethink---the `ToString()` functionality.  Like I said, I'll leave this open for a week for discussion, then merge in the change.  So if you disagree with anything I've written here, or you find the `ToString()` functions particularly useful, feel free to comment!  My perspective is not the only perspective. :)
You can view, comment on, or merge this pull request online at:

  https://github.com/mlpack/mlpack/pull/487

-- Commit Summary --

  * Remove ToString() from everything.
  * Update HISTORY.md.
  * Remove special handling for ToString().

-- File Changes --

    M HISTORY.md (2)
    M src/mlpack/core.hpp (1)
    M src/mlpack/core/dists/discrete_distribution.cpp (18)
    M src/mlpack/core/dists/discrete_distribution.hpp (5)
    M src/mlpack/core/dists/gaussian_distribution.cpp (17)
    M src/mlpack/core/dists/gaussian_distribution.hpp (6)
    M src/mlpack/core/dists/laplace_distribution.cpp (14)
    M src/mlpack/core/dists/laplace_distribution.hpp (3)
    M src/mlpack/core/dists/regression_distribution.cpp (18)
    M src/mlpack/core/dists/regression_distribution.hpp (5)
    M src/mlpack/core/kernels/cosine_distance.hpp (10)
    M src/mlpack/core/kernels/epanechnikov_kernel.cpp (11)
    M src/mlpack/core/kernels/epanechnikov_kernel.hpp (3)
    M src/mlpack/core/kernels/example_kernel.hpp (12)
    M src/mlpack/core/kernels/gaussian_kernel.hpp (9)
    M src/mlpack/core/kernels/hyperbolic_tangent_kernel.hpp (10)
    M src/mlpack/core/kernels/laplacian_kernel.hpp (9)
    M src/mlpack/core/kernels/linear_kernel.hpp (8)
    M src/mlpack/core/kernels/polynomial_kernel.hpp (10)
    M src/mlpack/core/kernels/pspectrum_string_kernel.hpp (14)
    M src/mlpack/core/kernels/spherical_kernel.hpp (9)
    M src/mlpack/core/kernels/triangular_kernel.hpp (9)
    M src/mlpack/core/math/range.hpp (6)
    M src/mlpack/core/math/range_impl.hpp (9)
    M src/mlpack/core/metrics/ip_metric.hpp (3)
    M src/mlpack/core/metrics/ip_metric_impl.hpp (11)
    M src/mlpack/core/metrics/lmetric.hpp (3)
    M src/mlpack/core/metrics/lmetric_impl.hpp (11)
    M src/mlpack/core/metrics/mahalanobis_distance.hpp (2)
    M src/mlpack/core/metrics/mahalanobis_distance_impl.hpp (24)
    M src/mlpack/core/optimizers/aug_lagrangian/aug_lagrangian.hpp (3)
    M src/mlpack/core/optimizers/aug_lagrangian/aug_lagrangian_function.hpp (3)
    M src/mlpack/core/optimizers/aug_lagrangian/aug_lagrangian_function_impl.hpp (12)
    M src/mlpack/core/optimizers/aug_lagrangian/aug_lagrangian_impl.hpp (13)
    M src/mlpack/core/optimizers/aug_lagrangian/aug_lagrangian_test_functions.cpp (8)
    M src/mlpack/core/optimizers/aug_lagrangian/aug_lagrangian_test_functions.hpp (3)
    M src/mlpack/core/optimizers/lbfgs/lbfgs.hpp (3)
    M src/mlpack/core/optimizers/lbfgs/lbfgs_impl.hpp (20)
    M src/mlpack/core/optimizers/sa/sa.hpp (2)
    M src/mlpack/core/optimizers/sa/sa_impl.hpp (24)
    M src/mlpack/core/optimizers/sdp/lrsdp.hpp (3)
    M src/mlpack/core/optimizers/sdp/lrsdp_function.hpp (3)
    M src/mlpack/core/optimizers/sdp/lrsdp_function_impl.hpp (14)
    M src/mlpack/core/optimizers/sdp/lrsdp_impl.hpp (10)
    M src/mlpack/core/optimizers/sgd/sgd.hpp (3)
    M src/mlpack/core/optimizers/sgd/sgd_impl.hpp (15)
    M src/mlpack/core/tree/ballbound.hpp (5)
    M src/mlpack/core/tree/ballbound_impl.hpp (15)
    M src/mlpack/core/tree/binary_space_tree/binary_space_tree.hpp (5)
    M src/mlpack/core/tree/binary_space_tree/binary_space_tree_impl.hpp (36)
    M src/mlpack/core/tree/cover_tree/cover_tree.hpp (5)
    M src/mlpack/core/tree/cover_tree/cover_tree_impl.hpp (35)
    M src/mlpack/core/tree/hrectbound.hpp (5)
    M src/mlpack/core/tree/hrectbound_impl.hpp (20)
    M src/mlpack/core/tree/rectangle_tree/rectangle_tree.hpp (5)
    M src/mlpack/core/tree/rectangle_tree/rectangle_tree_impl.hpp (35)
    M src/mlpack/core/tree/statistic.hpp (10)
    M src/mlpack/core/util/CMakeLists.txt (1)
    D src/mlpack/core/util/ostream_extra.hpp (37)
    M src/mlpack/core/util/prefixedoutstream.hpp (30)
    M src/mlpack/core/util/prefixedoutstream_impl.hpp (39)
    M src/mlpack/methods/cf/cf.hpp (5)
    M src/mlpack/methods/cf/cf_impl.hpp (13)
    M src/mlpack/methods/det/dtree.cpp (16)
    M src/mlpack/methods/det/dtree.hpp (5)
    M src/mlpack/methods/emst/dtb.hpp (5)
    M src/mlpack/methods/emst/dtb_impl.hpp (20)
    M src/mlpack/methods/fastmks/fastmks.hpp (5)
    M src/mlpack/methods/fastmks/fastmks_impl.hpp (18)
    M src/mlpack/methods/gmm/gmm.hpp (5)
    M src/mlpack/methods/gmm/gmm_impl.hpp (29)
    M src/mlpack/methods/hmm/hmm.hpp (5)
    M src/mlpack/methods/hmm/hmm_impl.hpp (12)
    M src/mlpack/methods/kernel_pca/kernel_pca.hpp (3)
    M src/mlpack/methods/kernel_pca/kernel_pca_impl.hpp (13)
    M src/mlpack/methods/kmeans/dual_tree_kmeans_statistic.hpp (12)
    M src/mlpack/methods/kmeans/kmeans.hpp (3)
    M src/mlpack/methods/kmeans/kmeans_impl.hpp (20)
    M src/mlpack/methods/kmeans/pelleg_moore_kmeans_statistic.hpp (10)
    M src/mlpack/methods/lars/lars.cpp (11)
    M src/mlpack/methods/lars/lars.hpp (3)
    M src/mlpack/methods/linear_regression/linear_regression.cpp (8)
    M src/mlpack/methods/linear_regression/linear_regression.hpp (3)
    M src/mlpack/methods/local_coordinate_coding/lcc.hpp (3)
    M src/mlpack/methods/local_coordinate_coding/lcc_impl.hpp (10)
    M src/mlpack/methods/logistic_regression/logistic_regression.hpp (3)
    M src/mlpack/methods/logistic_regression/logistic_regression_impl.hpp (10)
    M src/mlpack/methods/lsh/lsh_search.hpp (3)
    M src/mlpack/methods/lsh/lsh_search_impl.hpp (13)
    M src/mlpack/methods/nca/nca.hpp (3)
    M src/mlpack/methods/nca/nca_impl.hpp (13)
    M src/mlpack/methods/nca/nca_softmax_error_function.hpp (3)
    M src/mlpack/methods/nca/nca_softmax_error_function_impl.hpp (12)
    M src/mlpack/methods/neighbor_search/neighbor_search.hpp (3)
    M src/mlpack/methods/neighbor_search/neighbor_search_impl.hpp (24)
    M src/mlpack/methods/pca/pca.cpp (10)
    M src/mlpack/methods/pca/pca.hpp (3)
    M src/mlpack/methods/radical/radical.cpp (12)
    M src/mlpack/methods/radical/radical.hpp (3)
    M src/mlpack/methods/range_search/range_search.hpp (3)
    M src/mlpack/methods/range_search/range_search_impl.hpp (18)
    M src/mlpack/methods/rann/ra_search.hpp (3)
    M src/mlpack/methods/rann/ra_search_impl.hpp (45)
    M src/mlpack/methods/sparse_coding/sparse_coding.hpp (3)
    M src/mlpack/methods/sparse_coding/sparse_coding_impl.hpp (13)
    M src/mlpack/tests/CMakeLists.txt (1)
    D src/mlpack/tests/to_string_test.cpp (533)

-- Patch Links --

https://github.com/mlpack/mlpack/pull/487.patch
https://github.com/mlpack/mlpack/pull/487.diff

---
Reply to this email directly or view it on GitHub:
https://github.com/mlpack/mlpack/pull/487
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mailman.cc.gatech.edu/pipermail/mlpack-git/attachments/20151201/0ba7fbcc/attachment-0001.html>

More information about the mlpack-git mailing list