Repository navigation
docstring.py assumes that a method of class should NOT return a value #1123
Description
Activity
I understand that all methods of all classes in STUMPY main branch currently return nothing ("None"). However, I think it is not unreasonable to assume that, in future, a class might be added that has a method that returns a value.
I think that the
__call__is still an extreme example simply because we are trying to make the class "callable" (which is nice and I like this!). Technically, we have many, many class methods in STUMPY that return a value. For example:Lines 364 to 382 in 6f0169a
However, notice that while there is a clear return value in the method, we've simply but:
Returns ------- Nonein the Docstring. So, the class methods do and can return non-
NoneBUT the Docstring isn't reflecting the correct reality!So, maybe we should revise the else part in the code above ??
If I understand correctly, the current
elsehandles docstrings in classes and, even though some class methods have aReturnssection (some like thestimpclass have a return value ofNone), theReturnandNoneare still getting shoved intoparams_section(this is true even without thesdpclass!).Frankly, I cannot recall why I differentiated between aclassand a non-class because the two regexes look very similar except the non-class condition has an additional(?=Returns)at the end that tells the regex where to stop looking while theclasscondition keeps searching the entire docstring. There must be a rational reason for this!It looks like
docstring.pyrecognizes a class'def __init__(self,...)method as a valid function and attempts to read it.__init__methods rarely ever have return values so we stop searching for parameters by scanning all the way until the end of the docstring. Based on this observation, the key question is "Given a method/function docstring, when precisely should we stop searching for parameters?". In the case of a function, theParameterssection is always proceeded IMMEDIATELY by theReturnssection so we can safely stop and only capture the parameters. This is NOT true for class methods! Thus, for docstrings in class methods, we don't know (logically) when to stop searching the docstring besides reading until the end of the docstring!Perhaps, the simple solution is to ALWAYS add a
Returnssection to every class method and then we wouldn't need to differentiate between a function and a class method?we have many, many class methods in STUMPY that return a value. For example:
I missed that...probably because I was just looking at the "Returns" section of docstrings
However, notice that while there is a clear return value in the method, we've simply but:
Returns ------- Nonethe Return and None are still getting shoved into params_section (this is true even without the sdp class!).
Correct. And because it does not have that
:, the next line, i.e.args = re.findall(r"(\w+)\s+\:", params_section)gives the correct result.In the case of a function, the Parameters section is always proceeded IMMEDIATELY by the Returns section so we can safely stop and only capture the parameters. This is NOT true for class methods!
Perhaps, the simple solution is to ALWAYS add a Returns section to every class method and then we wouldn't need to differentiate between a function and a class method?
Maybe that was the initial intention behind adding "Returns" section to all methods of classes in STUMPY. I think the only case we need to handle is the method
__init__... even in that case, as you pointed out, if we add the section "Returns" , we should be fine (and that will cover the rare case__call__too)The whole if-else block can then be replaced with:
params_section = re.findall( r"(?<=Parameters)(.*)(?=Returns)", docstring, re.DOTALL )[0]Reacted by Sean M. Law- changed the title
[-]`doctoring.py` assumes that a method of class should NOT return a value[/-][+]`docstring.py` assumes that a method of class should NOT return a value[/+]on Jan 27, 2026 @NimaSarajpoor I will work on a quick PR for this!
Reacted by Nima Sarajpoor
If a method (of a class) returns a value, then the
elsepart in the following lines from docstring.py:stumpy/docstring.py
Lines 26 to 33 in ce05903
captures that output variable name from its docstring and includes it in the
args. This issue was initially exposed when a class with method__call__was added in PR #1118. One suggestion provided in this comment was:I think the real question we should ask ourselves is: "Should we treat methods like regular functions, meaning they can return value?
I understand that all methods of all classes in STUMPY
mainbranch currently return nothing ("None"). However, I think it is not unreasonable to assume that, in future, a class might be added that has a method that returns a value. So, maybe we should revise theelsepart in the code above ??