Documentation should present default values for optional parameters #2409

Open
alexandrejaguar opened this Issue Dec 19, 2016 · 5 comments

Projects

None yet

5 participants

@alexandrejaguar
Member

Hi all,
the documentation for all functions should present the default values for optional parameters. This does not happen for several functions. One example is #2407.

In feature.peak_local_max, exclude_border is:

    exclude_border : int, optional

It could be:

    exclude_border : int, optional (default : True)

I think we should have this for every function, thus helping the user to check this values when using the skimage library.

@jni
Contributor
jni commented Dec 20, 2016

@alexandrejaguar this is by design, because they can (in most cases) be determined by introspection of the function signature. However, I just saw that NumPy's how-to-document allows specification of defaults — I'd thought that it recommended against them. I think it does make things more readable not to have to glance back and forth between the docstring and the function signature, so I'd be in favour of adding these defaults.

@JDWarner
Contributor

I'm 👎 on this because it's redundant. IMO the best course would be to have NumPyDoc inspect the signature and pull in the info.

@alexandrejaguar
Member
alexandrejaguar commented Dec 20, 2016 edited

@JDWarner I think this would make defaults clear and readable on an eventual help: peak_local_max?. Actually, there are some functions in skimage with this:

  • morphology.remove_small_objects(): min_size, connectivity, in_place
  • segmentation.felzenszwalb(): multichannel

How would NumPyDoc work?

@b52
Contributor
b52 commented Dec 20, 2016 edited

Just a side note, the mentioned issue #2407 is problematic because of the default value True (which is listed in the signature and) which is not properly explained. Only the integer variant is properly documented and the rest is left unclear, so one has to consult the source code to understand what the boolean variant is about.

And for this particular parameter, the type signature should be something like int | bool, optional (Optional[Union[int, bool]]).

@alexandrejaguar
Member

@b52 you're right. I mentioned your issue because I was thinking on the documentation style in a while. It is unclear how to operate with the defaults on these functions at a glance. Several functions with a lot of parameters, such as peak_local_max(), would benefit from this.

@soupault soupault added this to the 0.14 milestone Dec 22, 2016
@soupault soupault referenced this issue Jan 12, 2017
Merged

Watershedline #2393

3 of 4 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment