TODO/FIXME comments in the code

[berndkr]

Description

For a university course I did an analysis of TODO/FIXME comments currently present in the Scikit-learn repository. I looked at the files in which the comments were and at the age of the comments. This gives 76 TODO comments in 48 files and 20 FIXME comments in 16 files:

Type	File	Date	Time passed (days)
TODO	/benchmarks/bench_plot_randomized_svd.py	19-08-15	570
TODO	/doc/developers/performance.rst	10-04-11	2162
TODO	/doc/developers/performance.rst	17-04-11	2155
TODO	/doc/developers/performance.rst	10-04-11	2162
TODO	/doc/developers/performance.rst	02-05-11	2140
TODO	/doc/modules/clustering.rst	10-09-11	2009
TODO	/doc/modules/gaussian_process.rst	12-11-10	2311
TODO	/doc/modules/svm.rst	02-09-10	2382
TODO	/examples/plot_johnson_lindenstrauss_bound.py	20-12-12	1542
TODO	/sklearn/cluster/k_means_.py	28-10-16	134
TODO	/sklearn/cluster/k_means_.py	19-12-11	1909
TODO	/sklearn/cluster/_k_means.pyx	09-10-12	1614
TODO	/sklearn/cluster/_k_means_elkan.pyx	27-04-15	684
TODO	/sklearn/datasets/mlcomp.py	02-09-11	2017
TODO	/sklearn/datasets/mlcomp.py	02-09-11	2017
TODO	/sklearn/datasets/svmlight_format.py	17-01-13	1514
TODO	/sklearn/decomposition/dict_learning.py	26-09-15	532
TODO	/sklearn/decomposition/dict_learning.py	26-09-15	532
TODO	/sklearn/decomposition/online_lda.py	11-09-15	547
TODO	sklearn/decomposition/tests/test_pca.py	21-03-15	721
TODO	/sklearn/externals/joblib/func_inspect.p	02-09-11	2017
TODO	/sklearn/externals/joblib/memory.py	02-09-11	2017
TODO	/sklearn/externals/joblib/memory.py	02-09-11	2017
TODO	/sklearn/externals/joblib/pool.py	20-12-13	1177
TODO	/sklearn/feature_selection/tests/test_feature_select.py	02-09-11	2017
TODO	/sklearn/feature_selection/univariate_selection.py	03-02-13	1497
TODO	/sklearn/grid_search.py	06-06-13	1374
TODO	/sklearn/linear_model/base.py	19-09-11	2000
TODO	/sklearn/linear_model/base.py	19-10-15	509
TODO	/sklearn/linear_model/base.py	19-10-15	509
TODO	/sklearn/linear_model/cd_fast.pyx	02-09-11	2017
TODO	/sklearn/linear_model/least_angle.py	02-09-11	2017
TODO	/sklearn/linear_model/least_angle.py	02-09-11	2017
TODO	/sklearn/linear_model/least_angle.py	02-09-11	2017
TODO	/sklearn/linear_model/least_angle.py	02-09-11	2017
TODO	/sklearn/linear_model/tests/test_base.py	06-11-15	491
TODO	/sklearn/linear_model/tests/test_least_angle.py	02-09-11	2017
TODO	/sklearn/linear_model/tests/test_ridge.py	21-03-15	721
TODO	/sklearn/linear_model/tests/test_ridge.py	21-03-15	721
TODO	/sklearn/linear_model/tests/test_ridge.py	06-11-15	491
TODO	/sklearn/linear_model/tests/test_sag.py	19-05-15	662
TODO	/sklearn/manifold/tests/test_isomap.py	27-04-12	1779
TODO	/sklearn/manifold/tests/test_locally_linear.py	21-04-12	1785
TODO	/sklearn/metrics/pairwise.py	12-01-15	789
TODO	/sklearn/metrics/tests/test_common.py	19-07-14	966
TODO	/sklearn/metrics/tests/test_common.py	19-07-14	966
TODO	/sklearn/metrics/tests/test_common.py	02-02-15	768
TODO	/sklearn/metrics/tests/test_score_objects.py	19-09-13	1269
TODO	/sklearn/model_selection/tests/test_search.py	16-06-16	268
TODO	/sklearn/neighbors/binary_tree.pxi	29-06-13	1351
TODO	/sklearn/neighbors/binary_tree.pxi	29-06-13	1351
TODO	/sklearn/neighbors/kde.py	29-06-13	1351
TODO	/sklearn/neighbors/kde.py	29-06-13	1351
TODO	/sklearn/neighbors/kde.py	29-06-13	1351
TODO	/sklearn/neighbors/kde.py	29-06-13	1351
TODO	/sklearn/neighbors/tests/test_neighbors.py	13-01-15	788
TODO	/sklearn/svm/base.py	02-09-11	2017
TODO	/sklearn/svm/libsvm.pyx	02-09-11	2017
TODO	/sklearn/svm/libsvm.pyx	02-09-11	2017
TODO	/sklearn/svm/libsvm_sparse.pyx	19-01-12	1878
TODO	/sklearn/svm/libsvm_sparse.pyx	19-01-12	1878
TODO	/sklearn/svm/libsvm_sparse.pyx	19-01-12	1878
TODO	/sklearn/svm/libsvm_sparse.pyx	19-01-12	1878
TODO	/sklearn/svm/src/libsvm/libsvm_helper.c	02-09-11	2017
TODO	/sklearn/svm/src/libsvm/libsvm_sparse_helper.c	02-09-11	2017
TODO	/sklearn/svm/tests/test_svm.py	02-09-11	2017
TODO	/sklearn/svm/tests/test_svm.py	02-09-11	2017
TODO	/sklearn/tests/test_naive_bayes.py	22-07-13	1328
TODO	/sklearn/utils/estimator_checks.py	18-07-14	967
TODO	/sklearn/utils/estimator_checks.py	07-04-15	704
TODO	/sklearn/utils/estimator_checks.py	07-04-15	704
TODO	/sklearn/utils/estimator_checks.py	18-07-14	967
TODO	/sklearn/utils/estimator_checks.py	18-07-14	967
TODO	/sklearn/utils/linear_assignment_.py	24-09-15	534
TODO	/sklearn/utils/murmurhash.pyx	17-01-12	1880
TODO	/sklearn/utils/src/cholesky_delete.h	03-01-13	1528
TODO	/sklearn/utils/_random.pyx	08-12-13	1189
FIXME	/doc/modules/linear_model.rst	08-07-11	2073
FIXME	/doc/tutorial/text_analytics/data/languages/fetch_data.py	17-05-13	1394
FIXME	/sklearn/cluster/hierarchical.py	25-07-13	1325
FIXME	/sklearn/cross_decomposition/pls_.py	24-03-15	718
FIXME	/sklearn/ensemble/gradient_boosting.py	30-05-14	1016
FIXME	/sklearn/ensemble/tests/test_gradient_boosting.py	27-07-13	1323
FIXME	/sklearn/externals/joblib/logger.py	02-09-11	2017
FIXME	/sklearn/externals/joblib/memory.py	02-09-11	2017
FIXME	/sklearn/linear_model/ridge.py	28-02-12	1838
FIXME	/sklearn/multiclass.py	02-09-11	2017
FIXME	/sklearn/neighbors/tests/test_kde.py	23-07-13	1327
FIXME	/sklearn/svm/classes.py	18-02-15	752
FIXME	/sklearn/svm/tests/test_svm.py	25-02-15	745
FIXME	/sklearn/tests/test_naive_bayes.py	12-12-14	820
FIXME	/sklearn/tests/test_naive_bayes.py	02-09-11	2017
FIXME	/sklearn/utils/deprecation.py	19-10-15	509
FIXME	/sklearn/utils/estimator_checks.py	07-04-15	704
FIXME	/sklearn/utils/estimator_checks.py	18-07-14	967
FIXME	/sklearn/utils/estimator_checks.py	19-03-15	723
FIXME	/sklearn/utils/estimator_checks.py	03-01-13	1528

The comments have an average age of 1390 days. With one striking example being a "FIX ME SOON!" comment added on Jan 3, 2013 (30045e161b88ada96f8d9aeb04e61). Which is still present in the current code (but in a different file). This shows that this might not be the best way to track these issues (and since there aren't many recent TODO/FIXME comments it seems like a practice that is no longer that acceptable in Scikit-learn). To avoid technical debt which contributors might not be aware of, it seems to me that it would be better to have these among the issues on GitHub to keep everything in a central location. Therefore I propose the following workflow:

1. Create an issue on GitHub for each file (python file, above table also contains a few /doc/ files were it might be acceptable to keep the TODOs) still containing TODOs/FIXMEs
2. File by file create issues for each TODO/FIXME comment
3. Either remove the comments in a new PR linking the newly created issues or if it is preferable to keep the comments in the code as well, instead of removing the comment add a link to the issue in the comment.

If you agree that this would be useful to do and also agree on the steps to solve it then this is something I would like to work on.

[lesteve]

Kudos for taking the time to open such a detailed issue! This shows a lot of motivation from your part.
I think your time and energy will be better used for the project if you try to tackle existing issues, e.g. the ones with a "bug" label.

If you agree that this would be useful to do

Basically this seems like a lot of tedious work for very little benefit. I am going to close this one. Feel free to reopen/shout if you strongly disagree.

[jnothman]

I don't think it would hurt for someone to take a look at these, but I'm not sure each deserves an issue at this point. Some are likely to be stale and can be removed easily, others are likely to need fixing. Some may already have issues. But I'd expect that if the code deserved that kind of remark, few of these would be "easy" issues, and we're lacking in expert time to handle more complex issues so it's not worth littering the issue tracker with them.

…

On 14 Mar 2017 8:45 pm, "Loïc Estève" ***@***.***> wrote: Kudos for taking the time to open such a detailed issue! This shows a lot of motivation from your part. I think your time and energy will be better used for the project if you try to tackle existing issues, e.g. the ones with a "bug" label. If you agree that this would be useful to do Basically this seems like a lot of tedious work for very little benefit. I am going to close this one. Feel free to reopen/shout if you strongly disagree. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#8581 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAEz6_f1mVIlaDiu-JONEaPdnD_qRXt1ks5rlmHTgaJpZM4Mbd-9> .