Add Google Style Docstrings

[FabioRosado]

We should implement Google Style Docstrings to every function, method, class in opsdroid. This style will support existing documentation and will help in the future by generating documentation automatically.

This consists in a bit of effort so this issue can be worked by more than one contributor, just make sure that everyone knows what you are working on in order to avoid other contributors spending time on something that you are working on.

If you are unfamiliar with the Google Style Docstrings I'd recommend that you check these resources:

• Sphix 1.8.0+ - Google Style Docstrings

Docstrings that need to be updated:

• main.py
  • configure_lang
  • configure_log
  • get_logging_level
  • check_dependencies
  • print_version
  • print_example_config
  • edit_files
  • welcome_message
• helper.py
  • get_opsdroid
  • del_rw
  • move_config_to_appdir
• memory.py
  • Memory
  • get
  • put
  • _get_from_database
  • _put_to_database
• message.py
  • Message
  • init
  • _thinking_delay
  • _typing delay
  • respond
  • react
• web.py
  • Web
  • get_port
  • get_host
  • get_ssl_context
  • start
  • build_response
  • web_index_handler
  • web_stats_handler
• matchers.py
  • match_regex
  • match_apiai_action
  • match_apiai_intent
  • match_dialogflow_action
  • match_dialogflow_intent
  • match_luisai_intent
  • match_rasanlu
  • match_recastai
  • match_witai
  • match_crontab
  • match_webhook
  • match_always
• core.py
  • OpsDroid
  • default_connector
  • exit
  • critical
  • call_stop
  • disconnect
  • stop
  • load
  • start_loop
  • setup_skills
  • train_parsers
  • start_connector_tasks
  • start_database
  • run_skill
  • get_ranked_skills
  • parse
• loader.py
  • Loader
  • import_module_from_spec
  • import_module
  • check_cache
  • build_module_import_path
  • build_module_install_path
  • git_clone
  • git_pull
  • pip_install_deps
  • create_default_config
  • load_config_file
  • envvar_constructor
  • include_constructor
  • setup_modules_directory
  • load_modules_from_config
  • _load_modules
  • _install_module
  • _update_module
  • _install_git_module
  • _install_local_module

---- ORIGINAL POST ----
I've been wondering about this for a while now and I would like to know if we should replace/update all the docstrings in opsdroid with the Google Style doc strings.

I think this could help new and old contributors to contribute and commit to opsdroid since the Google Style docstrings give more information about every method/function and specifies clearly what sort of input the function/method expects, what will it return and what will be raised (if applicable).

The downsize of this style is that the length of every .py file will increase due to the doc strings, but since most IDE's allow you to hide those fields it shouldn't be too bad.

Here is a good example of Google Style Doc strings: Sphix 1.8.0+ - Google Style Docstrings

I would like to know what you all think about this idea and if its worth spending time on it.

[jacobtomlinson]

Yes we should definitely do this!

It can also be really useful for automatically generating reference documentation.

[FabioRosado]

Awesome I'll wait a few days to see if anyone is opposed to this idea or if they would like to give some advice/comment on the issue.

If in a few days no one says anything I'll edit this issue just to explain more in depth what we expect of the comments and how to do it - I'd recommend dividing opsdroid per each .py file so different people can contribute to the issue 😄

[go8ose]

I like the idea. Is it possible to add a test that inspects the doc strings, and fails if they don't match the format? If so, would Jacob be happy with test coverage "reducing" in the short term as this test was added, but before all the doc strings complied?

…

On 29 April 2018 2:42:05 am AEST, "Fábio Rosado" ***@***.***> wrote: Awesome I'll wait a few days to see if anyone is opposed to this idea or if they would like to give some advice/comment on the issue. If in a few days no one says anything I'll edit this issue just to explain more in depth what we expect of the comments and how to do it - I'd recommend dividing opsdroid per each .py file so different people can contribute to the issue 😄 -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: #532 (comment)

-- Sent from my Android device with K-9 Mail. Please excuse my brevity.

[jacobtomlinson]

Yes I'm happy with that approach.

I've been thinking about it and I think there are two reasons why anyone would want to do this. The first is for autogenerating documentation, the second is making it easier for people to contribute.

As you said you are intending this to help people contribute, and I definitely agree. I just want to be clear on why we are doing this beyond it just being a thing that some projects do.

We currently run pydocstyle as part of the lint suite. I wonder if there is a way of telling it to enforce this docstring style?

[FabioRosado]

I'm not sure if it's possible to enforce google doc style in the lint, but I know that you can run tests on the docstrings like @go8ose suggested, Sphinx has a command for this (it uses the doctest module), but this tests might provide some issues and headaches.

The doctests will use the string representation present in the docstring to run the tests, if the result is not consistent like... a function that deals with dates for example and uses date.now() this test will always fail.
Another example would be running doctests with dictionaries, these tests will mostly fail due to the unsorted nature of dicts, the only way to make them pass would be to sort the dict all the time.

One way to work around it would be to just test some docstrings and not others. In Sphinx you can just add the command:

..doctest::
  >>> foo()
  bar

Finally, I believe that all the tests that we have at the moment do a very good job at testing every single piece of code in opsdroid so perhaps adding the doctests would be extra effort for no real gain - these will test what it's being tested already.

--EDIT--

I've updated my first post with all the functions,classes and methods that need to be updated, let me know if you need some added in or removed 👍

[go8ose]

Hi Fabio, I'm not suggesting we add more tests in the form of doctests. That would indeed be a waste of effort. I'm suggesting we check conformance with the google style doc strings. Jacob suggested seeing if this can be checked in the linting run. That is a good idea, linting is what I should have suggested initially. Cheers, Geoff

…

On 30 April 2018 5:51:38 pm AEST, "Fábio Rosado" ***@***.***> wrote: I'm not sure if it's possible to enforce google doc style in the lint, but I know that you can run tests on the docstrings like @go8ose suggested, Sphinx has a command for this (it uses the doctest module), but this tests might provide some issues and headaches. The doctests will use the string representation present in the docstring to run the tests, if the result is not consistent like... a function that deals with dates for example and uses date.now() this test will always fail. Another example would be running doctests with dictionaries, these tests will mostly fail due to the unsorted nature of dicts, the only way to make them pass would be to sort the dict all the time. One way to work around it would be to just test some docstrings and not others. In Sphinx you can just add the command: ``` ..doctest:: >>> foo() bar ``` Finally, I believe that all the tests that we have at the moment do a very good job at testing every single piece of code in opsdroid so perhaps adding the doctests would be extra effort for no real gain - these will test what it's being tested already. -- You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub: #532 (comment)

-- Sent from my Android device with K-9 Mail. Please excuse my brevity.

[jacobtomlinson]

This tool looks like it tests the docstrings against the google convention. We should explore it more.

https://github.com/terrencepreilly/darglint

[sims34]

Hi Fabio,
I like your idea, i new here on this project i can try to do something and make a pull request.
For my part i'm going to begin with helper.py it's Thk ok for you ?
Thk best regards

[FabioRosado]

Heya @sims34 yeah that would be much appreciated, let me know in gitter if you need any help with this 👍

[purvaudai]

Hi Fabio,
I am new here on this project. I was hoping I could help out with main.py
Regards

[jacobtomlinson]

Hey @purvaudai, please go ahead!

[FabioRosado]

Hello @purvaudai did you manage to work on main.py? If you are stuck with something let us know, we would be happy to help you getting started 👍

[mraza007]

Hi Guys is anyone working on this issue

[jacobtomlinson]

@mraza007 not currently. Please go ahead.

[mraza007]

Sure I will start working on this issue. Is there a way that you can assign this issue to me and on what files do I have to add google style doc string functions

[purvaudai]

Sorry for the lack of replies from my side. I tried solving this issue but got confused, so I decided to look through the docs again, and I got carried away learning. I am sorry for the lack of professionalism from my side.

…

On Mon, 25 Jun 2018 at 19:33, Muhammad ***@***.***> wrote: Sure I will start working on this issue. Is there a way that you can assign this issue to me and on what files do I have to add google style doc string functions — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#532 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AeXNt1F1hUU9JhsW2bl75KQG7SRQEceRks5uAO2_gaJpZM4TqkMs> .

-- Purva Udai Singh

[mraza007]

@purvaudai Hey are working on this issue do you want to continue

[NikhilRaverkar]

Hi guys I'd love to contribute too.
@mraza007 @purvaudai I know you guys are working on this, but if you need my help with any of the file, I'll be more than happy to contribute.

[jacobtomlinson]

@mraza007 @purvaudai @NikhilRaverkar Thanks for all wanting to contribute! There is a lot to be done on this issue so I strongly recommend picking a file and starting work on it.

Don't worry too much about duplicates, it's unlikely to happen given the number of methods that need updating. I would also be happy for you to submit lots of small PRs. Just pick a few methods, update the docstrings and raise a PR.

[mraza007]

Yup I think that would be a great idea

[FabioRosado]

@purvaudai Don't worry my friend sometimes these things happen, if you want to contribute to opsdroid in the future we will be glad to help you 👍

@mraza007 @NikhilRaverkar If you guys need any help, let us know. You can work on different files if you want and if something is not clear feel free to hit us up either in here or our gitter channel

[mraza007]

Sure I just joined the channel I will start working on this over the weekend

[NikhilRaverkar]

I’ll start working on this over the weekend. Thanks a lot, Nikhil

…

On Jun 28, 2018, at 10:44 AM, Muhammad ***@***.***> wrote: Sure I just joined the channel I will start working on this over the week — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

[jacobtomlinson]

@suparnasnair your PR was great. Welcome to open source! Please feel free to do more.

@p0larstern please go ahead.

@FabioRosado did you mean to close this?

[FabioRosado]

Ooops I didn't notice that the PR that I've merged would close the issue. I've reopened it haha

[lkcbharath]

Hi, I'm new to open source. Can I help out with adding docstrings in main.py if noone else is working on it?

[jacobtomlinson]

@lkcbharath please go ahead

[profwacko]

Hi @FabioRosado would there be a quick way for me to understand the functionality of each of the functions I'm writing docstrings for (in matchers.py)? Or maybe sort of bump me into the best direction for that.

[FabioRosado]

I'd say reading the matchers documentation might give you a better understanding of what each of them do. But basically matchers are decorators that you use to match a skill(python function) to an intent obtained from a parser.

For example on the dialogflow - the parser will call the API and send over the message from the user, then the API does its thing and returns a response which might contain an intent and other entities.

In dialogflow there is an intent called - smalltalk.greetings so you use the @match_dialogflow_intent('smalltalk.greetings') to trigger a skill. The matchers basically make that bridge between parsing a message and getting a result and then match a skill with said intent.

You can check the example on the dialogflow documentation for more information.

I hope this made sense?

[paqman85]

Hi @FabioRosado!
I'm looking to help out on this as well. Looks like the checklist may be out of date now? - I see there was a merge for the CORE section but still some outstanding checkboxes. Can you confirm if the checklist is up to date? I'd love to try and tackle any stragglers.

[FabioRosado]

Hello paqman unfortunately the checklist has been out of date for a while and I haven't have the time to fix it yet. It would be great if you could tackle some of them, Im off in 5 days so I'll try to update the list, you can wait until then or check each file and see if any is left without the google style docstrings - I know there is still a lot of them that don't have it

[profwacko]

Hi @FabioRosado, thanks for the tips. It's definitely more clear now. The checklist for matchers.py seems to be lacking a few more functions, namely:

• match_event
• match_parse
• match_sapcai
• match_watson

Also the following functions don't seem to be there anymore, i'm guessing they are now the dialogflow api ones?

• match_apiai_action
• match_apiai_intent

I'll fix up the docstrings for these as a starting point:

• match_event
• match_regex
• match_parse

[FabioRosado]

Yeah you were right the api ai matcher has now become dialogflow - i think I added the google style on them when i updated the version not sure though.

Those new ones aren't there because they were added after I created this issue, which reminds me that I forgot to update the list 👍

[profwacko]

@FabioRosado I hope my pull request is okay for the first few changes I did, please let me know if there's anything I can do to make the pull request better! I am also doing this for hacktoberfest 😄

[profwacko]

I'll claim a few more docstrings for matchers.py, i'll be doing the diaglogflow, crontab, webhook and always docstrings.

Here's the updated checklist for that:

• match_regex
• match_apiai_action
• match_apiai_intent
• match_dialogflow_action
• match_dialogflow_intent
• match_luisai_intent
• match_rasanlu
• match_recastai
• match_witai
• match_crontab
• match_webhook
• match_always
• match_event
• match_parse
• match_sapcai
• match_watson

[daniccan]

@FabioRosado I would like to contribute to this issue. Any file that I can pick up?

[jacobtomlinson]

At this point I would just have a look through files on master and see what doesn't have good docstrings.