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

Standard for variable names in Returns docstring section #3422

Open
MatthewFlamm opened this issue Oct 6, 2022 · 1 comment
Open

Standard for variable names in Returns docstring section #3422

MatthewFlamm opened this issue Oct 6, 2022 · 1 comment
Labels
maintenance Low-impact maintenance activity

Comments

@MatthewFlamm
Copy link
Contributor

MatthewFlamm commented Oct 6, 2022

Describe what maintenance you would like added.

This stems from the discussion from #3391 (comment).

I think the standard in the code base is to omit names for returned variables like below. But this is an undocumented standard, or I couldn't find it by reading the Contributing section.

Returns
-------
int
   Number of mesh points

I think the standard should be documented, but first we need to align on what the standard should be. I prefer to name the returned variables. This is standard in NumPy, although it doesn't look explicitly documented there. There are several positives, in my opinion, to this approach:

  • In ravel the return name is not important, but it is referenced in the description. Otherwise we would have to use descriptive words to describe that parameter each time we refer to it.
  • In linspace the return names allow to immediately know what each returned variable is in a single word. It also gives a suggestion for how to name the returned variables that breeds consistency in the community. These variable names are also referenced in the Parameters section that makes it clear how the Parameters interact with the Returns.

There is a big negative to this approach in that it would require a large effort to add this to the codebase. Another negative is that we probably end up with a lot of return names like out or result or y and these may not be needed to be referenced further.

@MatthewFlamm MatthewFlamm added the maintenance Low-impact maintenance activity label Oct 6, 2022
@akaszynski
Copy link
Member

akaszynski commented Oct 10, 2022

I can see both sides of the argument. I think we should pick one and document it.

I'm in favor of not returning the variable names unless there are multiple variables since (as pointed out), many of our variables do not have "useful" names.

  • In linspace the return names allow to immediately know what each returned variable is in a single word. It also gives a suggestion for how to name the returned variables that breeds consistency in the community. These variable names are also referenced in the Parameters section that makes it clear how the Parameters interact with the Returns.

I think this is precisely why we should name variables when we have multiple returns, and we should make that a hard requirement. However, I see less of a reason for this with a single variable return.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintenance Low-impact maintenance activity
Projects
None yet
Development

No branches or pull requests

2 participants