Improve attributes and methods section in docs about skorch HOT 10 CLOSED

We linked most of the methods and decided that line breaks are not an issue important enough to leave this ticket open.

from skorch.

Is this still something we would want to do before releasing 0.2.0?

from skorch.

I'm fine with leaving it open for now

from skorch.

@benjamin-work Not clear what exactly you were referring to here. Is this perhaps fixed with #213 and #242? Or if not, please point me where the issues are and I'll send another PR.

from skorch.

The issue with the links on methods seems to be fixed.

Regarding the long lines: Look here at the Methods section. At the top, there is a table that lists all methods with their docstrings but without line breaks. Below, the methods are described again in a more readable fashion. I believe the table should be removed.

from skorch.

The table lists all the methods, including inherited. This may be useful to get a quick overview of all what a particular class has to offer. (Although the inherited methods are not hyperlinked, this makes it far less useful.)

The lack of line breaks is inconvenient, however it feels like the true issue is that the function names include arguments. This makes the first column too long, leaving very little space for the second. I think if this was corrected, then almost all docstrings would fit without scrolling. (And arguably those that don't fit should be simplified to be more concise.)

That being said, it seems like we anyway have no control over any of these with automodule directive.

One option would be to ditch automodule and compose documentation in a semi-automatic way (e.g. list classes and methods explicitly, but let automethod add docstrings). This is how PyTorch does it. I'm not sure how desirable this is given that it complicates maintenance.

from skorch.

Overall, these deficits in the docs are not a huge issue, if it takes too much work to mend them, then it's better to let it be.

the function names include arguments

If that could be fixed, we would already gain a lot.

those that don't fit should be simplified to be more concise

We should consider this. Are there any standards on how long the first part should be?

from skorch.

If that could be fixed, we would already gain a lot.

Again, no option is exposed to control that. Perhaps we can inject some javascript in the page template that would hide the arguments inside summary tables. If this is something you may consider, I can explore this possibility.

Are there any standards on how long the first part should be?

PEP 257 suggests one line:

The summary line may be used by automatic indexing tools; it is important that it fits on one line.

So does Google style guide:

A doc string should be organized as a summary line (one physical line) terminated by a period.

... and NumPy style guide:

A one-line summary that does not use variable names or the function name.

By the way, NumPy style (which skorch probably should follow being a part of scikit family) stipulates 75 characters long docstrings. From my observations, skorch docstrings seems to be formatted for 70 characters width.

from skorch.

The summary line may be used by automatic indexing tools; it is important that it fits on one line.

This is an ideal that is probably hard to keep up with in practice, but we should try.

skorch docstrings seems to be formatted for 70 characters width.

I believe this is something we don't enforce consciously yet.

In general, we may consider using black in the future for code formatting, once it becomes stable, especially since our code is mostly compliant anyway. However, I didn't see anything about line length in docstrings.

If this is something you may consider, I can explore this possibility.

I believe we have more urgent things to do at the moment. And I wouldn't want to be the one debugging javascript if it were to fail one day ;)

from skorch.

FWIW, this one-liner removes the arguments:

$('dl.class table.longtable.docutils td:first-child').contents().filter(function(){return this.nodeType === 3;}).remove()

Of course, this will break if Sphinx decides to change CSS class names, so I understand your reluctance to integrate a fix of this kind.

I suppose there are no actionable items in this issue then (besides from perhaps branching out another issue for future code formatting).

from skorch.