Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Back-end code style discussion #2307

Closed
jnm opened this issue Jul 2, 2019 · 9 comments
Closed

Back-end code style discussion #2307

jnm opened this issue Jul 2, 2019 · 9 comments
Assignees
Labels

Comments

@jnm
Copy link
Member

jnm commented Jul 2, 2019

Current approach

Use type hinting and write docstrings that are helpful to humans. From discussion below:

If we're just writing for other humans, though, then I'd prefer to drop the boilerplate stuff.

I think I lean more this way. After looking at a couple big projects that use annotations, most of them don't include that boilerplate and rather just have a description and maybe something about what's returned. Some also include what kind of error response could be expected which is nice.

So something more like:

def method(something: str) -> bool:
    """
    Return True if `something` is equal to 'nothing'
    """
    return something == 'nothing'

Outdated

Internal discussion: https://chat.kobotoolbox.org/#narrow/stream/4-KoBo-Dev/topic/Python.20coding.20style

Let's use this type of documentation going forward:

    def get_submission_edit_url(self, submission_pk, user, params=None):
        """
        Gets edit URL of the submission from `kc` through proxy
        
        Args:
            submission_pk (int)
            user (User)
            params (dict): passed to the KC request
            
        Returns:
            dict: extra parameters to include in the KoBoCAT GET request
        """

…and let's update existing code as we're able.

According to https://sphinxcontrib-napoleon.readthedocs.io/en/latest/example_google.html,

    The format for a parameter is::

        name (type): description
            The description may span multiple lines. Following
            lines should be indented. The "(type)" is optional.

            Multiple paragraphs are supported in parameter
            descriptions.
@jnm
Copy link
Member Author

jnm commented Feb 15, 2021

@noliveleger, @joshuaberetta, @JacquelineMorrissette, this kind of stuff seems obsolete now that there's type hinting:

        Args:
            submission_pk (int)
            user (User)

Has anybody seen Python 3 docstring guidance that they like?

@noliveleger
Copy link
Contributor

noliveleger commented Feb 15, 2021

What about going back to old way

def method(param1: str, param2: list) -> bool:
    """
    Description of the method
    
    :param param1: Description for param1
    :param param2: Description for param2
    :return: Description for return (useless in that case)
    """
    value = False
    ...
    return value
    

@jnm
Copy link
Member Author

jnm commented Mar 1, 2021

Does the :param …: stuff let us use an automatic documentation builder? (Sphinx?)
If so and we want to follow through with building that kind of documentation, let's go for it. If we're just writing for other humans, though, then I'd prefer to drop the boilerplate stuff.

@noliveleger
Copy link
Contributor

@JacquelineMorrissette
Copy link
Contributor

It looks like the :fieldname: description does work with auto-documentation builders. Though for Sphinx specifically, it doesn't look like :param param1: description is really necessary, for that to work. My reference: https://www.sphinx-doc.org/en/master/usage/restructuredtext/field-lists.html

I can't say I have a preference for any docstring guides.
But using the one using args:... seems a lot cleaner from the two presented, but with type hinting, I agree that having the type in there is obsolete

@joshuaberetta
Copy link
Member

joshuaberetta commented Mar 4, 2021

If we're just writing for other humans, though, then I'd prefer to drop the boilerplate stuff.

I think I lean more this way. After looking at a couple big projects that use annotations, most of them don't include that boilerplate and rather just have a description and maybe something about what's returned. Some also include what kind of error response could be expected which is nice.

So something more like:

def method(something: str) -> bool:
    """
    Return True if `something` is equal to 'nothing'
    """
    return something == 'nothing'

@noliveleger
Copy link
Contributor

The boilerplate is used by IDEs to explain each parameter. Modern IDEs do not only use 70% of your memory for nothing. They are capable of such things :-D

Ok, Let's remove the boilerplate, use human readable description.
@jnm. Would you mind to update your post then?

@joshuaberetta
Copy link
Member

😂

Modern IDEs do not only use 70% of your memory for nothing

My poor laptop struggles enough with the VM running and a browser open that I can barely open anything else 💀 Sometimes even vim struggles 😂

@jnm jnm changed the title Use Google-style Python docstrings Back-end code style discussion Jul 20, 2021
@jnm jnm closed this as completed Dec 1, 2022
@jnm jnm unpinned this issue Dec 1, 2022
@jnm
Copy link
Member Author

jnm commented Dec 1, 2022

Updated description; closed; unpinned

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

4 participants