BPSB / combine-p-values-discrete

BSD 3-Clause "New" or "Revised" License
0 stars 0 forks source link

Suggestions for improving documentation #3

Closed steppi closed 8 months ago

steppi commented 8 months ago

On the whole I think this project is very well documented. The content is well written and explains the package well. I just have a few minor suggestions for improvement.

Wrzlprmft commented 8 months ago

On the whole I think this project is very well documented. The content is well written and explains the package well.

Thank you.

It would be nice if there were examples in the docstrings for the functions in the core API. Just very basic ones to show how the functions are used at a glance.

Done.

I like to look at documentation in the terminal, and I think this is a pretty common use case. I noticed the docstrings contain very long lines, […] which can lead to messy wrapping. I think it would be more user friendly if you wrapped them at around 90 characters.

I beg to disagree. If I manually wrap everything to 90 characters, it would be a pain to read with a terminal that has a width of 70 characters. Almost every environment I know (terminals, Jupyter, …) can auto-wrap docstrings decently (GitHub being one of the few exceptions). Moreover, manually wrapped docstrings are unfeasible for editing since you need to re-wrap the entire paragraph if you make a substantial change at the beginning. I am aware that I am in the minority with my opinion though. (I am also perplexed why programmers of all people insist on doing something manually that can be decently automatised since decades.)

steppi commented 8 months ago

I beg to disagree. If I manually wrap everything to 90 characters, it would be a pain to read with a terminal that has a width of 70 characters. Almost every environment I know (terminals, Jupyter, …) can auto-wrap docstrings decently (GitHub being one of the few exceptions). Moreover, manually wrapped docstrings are unfeasible for editing since you need to re-wrap the entire paragraph if you make a substantial change at the beginning. I am aware that I am in the minority with my opinion though. (I am also perplexed why programmers of all people insist on doing something manually that can be decently automatised since decades.)

I see. I won't force the issue if you want to keep it this way. Just to present my case, here is how it looks to me in a terminal using XFCE on fullscreen.

Screenshot_2024-02-18_10-39-10

I have mild dyslexia and these super long lines wrapped around in the middle of words are pretty difficult for me to read. Here's how it looks to me in a terminal in emacs side by side with a buffer of code, which is how I often work. This is also not easy for me to read.

Screenshot_2024-02-18_10-42-09

PEP8 suggests wrapping code in Python at 79 columns, and comments and docstrings at 72, which is a holdover from when 80 columns was the standard width for a terminal. I like this because I like to work with 2 or even 3 buffers side by side, but because 80 is an artificial restriction, the general trend seems to be to move Beyond PEP8, and go with around 90 columns. This is codified in the commonly used code formatter black which defaults to 88 characters. That's why I recommended around 90. The linter in SciPy's CI restricts things to 88 columns, so I couldn't in good faith suggest something less than that.

I use emacs' fill commands to wrap texts in Docstrings and it works pretty painlessly. I'm sure almost any editor or IDE has tooling that could do the same.

Wrzlprmft commented 8 months ago

Just to present my case, here is how it looks to me in a terminal using XFCE on fullscreen.

[…]

I have mild dyslexia and these super long lines wrapped around in the middle of words are pretty difficult for me to read. Here's how it looks to me in a terminal in emacs side by side with a buffer of code, which is how I often work. This is also not easy for me to read.

[…]

But that is all based on the assumption that you must display code like that, using the entirety of the window and wrapping it like that. I am pretty confident there are ways to set your terminal or Emacs to wrap text reasonably to a line width that you prefer. In particular for accommodating dyslexia and the like, manually wrapping text and PEP8 is harmful since it forces you to read code exactly as it was written and unnecessarily removes the flexibility to adjust it to your needs. It’s as unnecessary as dictating the font in which you view code.

For example, here is how the same section of code looks on two terminal editors with different widths for me:

wrapping

This is (at least for me) reasonably wrapped and readable and it adapts should I change the width of my terminals, e.g., by opening another terminal. With manually wrapped code, I would get a mess on at least one side, usually both. (Mind that here the width of the window dictates the width of the terminal output, but that’s a conscious choice. If I have text in a single terminal and it’s too wide for me, I just open a second one and the problem is solved.)

I use emacs' fill commands to wrap texts in Docstrings and it works pretty painlessly. I'm sure almost any editor or IDE has tooling that could do the same.

This assumes that wrapping can be done and undone completely automatically. While this holds for wrapping (at least if you use protected spaces where applicable), it doesn’t for unwrapping because it cannot distinguish a linebreak made for wrapping from one that carries meaning. Now if you accept automatic wrapping, why not wrap on the reader’s side instead of the writer’s? That way you do not have any superfluous changes of the source and you avoid the inevitable imperfections of automatic unwrapping.

steppi commented 8 months ago

You can keep it this way. I think because yours is the first project I’ve seen with this convention I’ve never thought look for a solution for viewing docstrings, man pages, etc. with extremely long lines. I’m kind of curious about your set up though. What do you use for editing code with very long lines to do the intelligent wrapping? Does it handle indentation? Does it work for very large files without getting sluggish? It could be useful for me.

Wrzlprmft commented 8 months ago

What do you use for editing code with very long lines to do the intelligent wrapping? Does it handle indentation? Does it work for very large files without getting sluggish? It could be useful for me.

I use Neovim with some plugins that I set up years ago and never touched since. I freely admit that I do not know what is responsible for the wrapping and would have to dig into it myself. I haven’t experienced any problems with large files so far, but then I may not have handled whatever is a large file for you.

steppi commented 8 months ago

Thanks. I've figured out how to do it with Emacs too. Looks better now.

Screenshot_2024-02-20_12-14-46