Skip to content

Configuring the documentation for Sphinx and ReadTheDocs#40

Open
joe-from-mtl wants to merge 40 commits intomainfrom
sphinx-config
Open

Configuring the documentation for Sphinx and ReadTheDocs#40
joe-from-mtl wants to merge 40 commits intomainfrom
sphinx-config

Conversation

@joe-from-mtl
Copy link
Copy Markdown
Contributor

@pep8speaks
Copy link
Copy Markdown

pep8speaks commented Dec 17, 2024

Hello @joe-from-mtl! Thanks for updating this PR.

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2024-12-17 21:22:34 UTC

@joe-from-mtl joe-from-mtl marked this pull request as ready for review December 17, 2024 19:51
@joe-from-mtl joe-from-mtl requested a review from CHrlS98 December 17, 2024 19:51
return npz['metadata'][0], npz['types'][0]['metadata']


def _example_one():
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What are those?

Attributes
----------
directory : str

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Attribute description?


Parameters
----------
img1
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing : no?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you fix the linum_detect_focalCurvature.py too?

Co-authored-by: Charles Poirier <41654474+CHrlS98@users.noreply.github.com>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should also mention somewhere than the zarr chunk size is often chosen to match the raw tile size. This is important for the processing pipeline.

@FIrgolitsch
Copy link
Copy Markdown
Contributor

FIrgolitsch commented May 26, 2025

I was looking at this for my own additions, but I think we should agree on a style first. For LIOM Toolkit I have been using docstrings like this for readthedocs:

def get_folder_list(tiles_directory: Path) -> list[Path]:
    """
    List all the tiles in a given directory.

    :param tiles_directory: Path to the directory containing the tiles.
    :type tiles_directory: Path
    :return: List of tile file paths.
    :rtype: list[Path]
    """

I based this off of the Sphinx-RTD documentation:
https://sphinx-rtd-tutorial.readthedocs.io/en/latest/docstrings.html

It's a bit less verbose than what we have, but I prefer this as it leaves a lot less whitespace compared to:

def generate_default(nX, nY):
    """Generates a default topology where all tiles in mosaic are nodes and all neighbor relation is an edge.
    Parameters
    ----------
    nX: int
        Number of tiles in X direction.
    nY: int
        Number of tiles in Y direction.

    Returns
    -------
    mosaicTopo : NetworkX graph object describing the mosaic topology

    Note
    ----
    Each node position (in tile reference) can be accessed as node attributes 'x' and 'y'.

    """

@CHrlS98
Copy link
Copy Markdown
Contributor

CHrlS98 commented May 29, 2025

@FIrgolitsch can you add a comment to this issue #59 . Let's brainstorm there for what coding standards we want to follow

Also, in scilpy we use the second option so it is what I am used to, but I wouldn't mind going for the first option. We only need to be constant across the codebase.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants