`dyc diff` fails when removing docstrings from a method

[Zarad1993]

Removing docstrings from a method should still make dyc diff work. If we remove that at the moment it will treat the code like it's documented.

[Abo7atm]

Hey @Zarad1993, I'm a first time contributor and would like to work on this. Could you please explain this issue a bit more?

[Zarad1993]

Hey @Abo7atm - Thank you so much for choosing this project 🙂 !

At a high level,

dyc diff should work to read your additions in (git diff) patch, if any new methods were added. Then it prompts the requests for adding Docstrings. On the other hand, when you remove a patch of code. The diff parameter does not read your deletions and the method you removed the git diff from.

To reproduce:

def hello_world():
    """
        My docstrings
    """
    return 'hello world'

1. commit snippet above.
2. Remove docstrings
3. Run git diff
4. Expectation: evaluate the method docstrings were removed from and prompt the dev to document the method.

Please let me know if further details are needed.

[Abo7atm]

I tried your example on my machine, and it seems that dyc diff fails when the triple double -or single, for that matter- quotes aren't removed. If I remove them, dyc diff would detect the removal of the docstring, and would prompt me to document the method. I think the command should check if the docstring is empty before skipping the method.

I read the method in the code that checks whether the method has been documented or not, and from what I understood, the method is_first_line_documented in methods.py is the culprit. The behavior expected requires to check whether there's documentation between the triple double quotes or not.

What do you think would be a good approach to this? I'm working on it right now.

PS. if you could tell me how can I run the package from the source code, not the pip version, so I can evaluate the changes I'm making, I'd be thankful.

[Zarad1993]

I think you're on the right track. The method is the evaluator, however, when running diff the patch is brought from git. AFAIK, only the additions are passed to MethodBuilder and not deletions.

Check development for setting it up locally. Happy to help further if needed.

[Abo7atm]

I was thinking about an approach to verify whether a method has a docstring or not. One idea that comes to mind that utilizes the MethodInterface class is using regex. We could look for a (open)((\s|\n)*(\w|\d)+(\s|\n)*)+(close) pattern in the method's body, instead of checking the first line for the opening quotes. This can also detect whether a single line docstring is empty or not.

[nilesh-kaizen]

Hi @Zarad1993, this would be my first contributing to any project so please let me know if I am wrong somewhere, I looked through the code and found out that when we remove the comments and only blank quotes are present Ex. """ """ from """ Comments about method """ git diff doesn't find anything as changed and in dyc.diff() method being diff_only=True it is not called. Changing diff_only=False along with some changes in method.py I am able to get the prompt to add a comment when I am deleting the comments and having only """ """ in my methods.

Please let me know if it is a correct approach or diff_only should remain true.

[Zarad1993]

Hi @nilesh-kaizen - I'm not sure of the potential side effects for this, yet. Feel free to open a PR and add some tests for it to proof case your fix.

Thanks

[Marinette]

Hello, is anyone still working on this?

[Zarad1993]

Hi @Marinette - You are welcome to look at it. I think it's been a while and there isn't a PR out yet.