Move docs from __init__ to right after class defiition

[adam-narozniak]

Issue

Some strategies have comprehensive docstrings in __ init__ but also short docstrings just after the class definition. That approach is not consistent with PEP 257 and it leads to documentation doubling.

Description

PEP 257 says that the class Docstrings should be (immediately following the class definition) - the most common option in our docs. The constructor (__init __) Docstrings is optional

Related PRs

#2644 describes the issue from the documentation POV.

Proposal

Move the docs from __ init__ to the right after the class def. This problem concerns Strategies (the rest of the code places the docstrings correctly).

[charlesbvll]

All of my comments are towards the old docstrings, but I feel like it's the right opportunity to make those fixes! Otherwise LGTM!

[adam-narozniak]

@charlesbvll Thanks, good points. Let's make it consistent now.

What do you think about removing the "default from the docs"?

Re the links to the papers I think the best would be not to point directly to pdf but the arxiv.org/sth (there they can also see version etc). wdyt?

[charlesbvll]

@charlesbvll Thanks, good points. Let's make it consistent now.

What do you think about removing the "default from the docs"?

Re the links to the papers I think the best would be not to point directly to pdf but the arxiv.org/sth (there they can also see version etc). wdyt?

@adam-narozniak Looking at our docs, it seems like Sphinx does not indicate the default value in all cases, which means that removing it from the docstring might remove some useful information from the docs. If we find a way to make it work consistently without using the docstring, I agree that it would be ideal. It's would be in another PR anyways.

For the links, they're all pointing to the abstract instead of the pdf (except the one I pointed out), so it should be good!

[adam-narozniak]

Ok, so I'm gonna fix the defaults in the docs, but then I think it'd be good to get rid of them. Sphinx shows the defaults in the parameters (in the function/class signature). What's the example that does not have that?

[charlesbvll]

Ok, so I'm gonna fix the defaults in the docs, but then I think it'd be good to get rid of them. Sphinx shows the defaults in the parameters (in the function/class signature). What's the example that does not have that?

It might be an issue with classes, here for instance the default for seed and shuffle are not shown (I took an example out of FDS because the goal is to migrate to the same tool). You can also see it on FedAvg or other strategies.

[adam-narozniak]

Got it. I think that it's not an issue if that's not present in the "parameter doc" (for the lack of a better term).
But it will be present in the signature in the case of a function/method and constructor is the case of a class, and it's present in the example you gave. By a constructor I mean

class FederatedDataset(*, dataset: str, subset: str | None = None, resplitter: Callable[[DatasetDict], DatasetDict] | Dict[str, Tuple[str, ...]] | None = None, partitioners: Dict[str, [Partitioner](https://flower.dev/docs/datasets/ref-api/flwr_datasets.partitioner.Partitioner.html#flwr_datasets.partitioner.Partitioner) | int], shuffle: bool = True, seed: int | None = 42)

so e.g. seed = 42
and it'll be the same in case of function/method (I don't think there's any function in public API in FDS; but for method), by signature I mean

load_partition(node_id: int, split: str | None = None)

the split = None is present.

So if we wanted the defaults in the "parameter docs" I would say it would be really worth it if there's some extension to do that. Because the cost of getting it wrong e.g. not changing manually when the change is in the code, outweighs the lack of having it in the parms docs because it's still in the signature/constructor. That's how I see it.

The issue of default values visibility should be alleviated/less prominent when we have constructor/signature docs such that there's one parameter per line or the types are stripped from the constructor/signature. The current formatting makes it hard to read. That's one of the problems I see with the current docs.