Closed matteodelabre closed 6 years ago
Thank you for the PR! The changes look good, can you please instead of a compile-time option just add a runtime option? We could either put it through with a parameter or by reading an environment var for activation.
I am aware about the (minimal) runtime costs, but if you say we should avoid them altogether we can just remove my log outputs and add a little executable that outputs the same noisy details. I just used those during research.
Thanks for the feedback. I agree that a runtime option would be more convenient than a compile-time option. However, as you mentioned, it would create a small runtime cost for end-users that do not care about the logging instructions.
For these reasons, I think creating a separate executable, as you proposed, would be the best tradeoff, especially given that this information is most likely to be used only internally, while debugging reading problems.
I set up an executable called dump
in the latest commit that does just that. Let me know what you think.
All done! Regarding the formatting, the intent was to visually group together lines in the code that output a single line in the terminal, but it turns out it is not that readable.
Currently, the notebook decoder outputs various debugging information to the standard output while decoding files. While this output can be useful at times, it is, in my opinion, unnecessary for most users of the library. The proposed changes include: