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

Multiline docstrings fix #2130

Merged
merged 33 commits into from
Apr 3, 2024
Merged

Conversation

sharsha315
Copy link
Contributor

@sharsha315 sharsha315 commented Mar 23, 2024

This PR focuses on formatting multiline docstrings using the Google style guide for consistency and clarity across the project. The changes made to autogen/autogen/agentchat/contrib/retrieve_user_proxy_agent.py and autogen/autogen/agentchat/contrib/retrieve_user_proxy_agent.py partially address the issue #2048, as many files in the project require similar formatting adjustments. Please review and merge accordingly.

Why are these changes needed?

  1. Added a first single line on RetrieveUserProxyAgent. The short definition line was taken by referring [here.]https://microsoft.github.io/autogen/blog/2023/10/18/RetrieveChat#introduction)
    first_single_line

  2. Formatted Docstrings in the RetrieveUserProxyAgent Class in autogen/autogen/agentchat/contrib/retrieve_user_proxy_agent.py file.
    formatted_docstrings_1

  3. Formatted docstrings for the initiate_chats functiion of ChatResult class in autogen/agentchat/chat.py file.
    formatted_docstrings_2

Related issue number

#2048

Checks

…nd Added first single line for the class RetrieveUserProxyAgent.
@sharsha315
Copy link
Contributor Author

sharsha315 commented Mar 23, 2024 via email

@ekzhu
Copy link
Collaborator

ekzhu commented Mar 23, 2024

Have you tried to build the website locally and see what the results look like in the API docs? See contrib docs for building website locally. You can take a screenshot of that much appreciated

@gagb
Copy link
Collaborator

gagb commented Mar 23, 2024

Have you tried to build the website locally and see what the results look like in the API docs? See contrib docs for building website locally. You can take a screenshot of that much appreciated

Thank you for the PR!

To add to what Eric said. Also might wanna check if the doc string renders correctly in the IDE. Before/after screenshots would be nice. I think @jackgerrits recommended the google convention? It would be nice to update the description to say which convention you used.

Also closing #2048 would require edits to a lot of files.

@sharsha315
Copy link
Contributor Author

Have you tried to build the website locally and see what the results look like in the API docs? See contrib docs for building website locally. You can take a screenshot of that much appreciated

Thank you for the PR!

To add to what Eric said. Also might wanna check if the doc string renders correctly in the IDE. Before/after screenshots would be nice. I think @jackgerrits recommended the google convention? It would be nice to update the description to say which convention you used.

Also closing #2048 would require edits to a lot of files.

Thanks for the feedback! I initially focused on fixing the two files mentioned in the issue. You said it would require edits to a lot of files, could you please clarify which files I'm allowed to change and which files allowed not to change. Also, I'll carefully look into your recommendation and update the description accordingly. Your guidance would be greatly appreciated, so that I can continue working on it. Thanks again.

@codecov-commenter
Copy link

codecov-commenter commented Mar 24, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 37.94%. Comparing base (6c3d779) to head (227b063).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2130   +/-   ##
=======================================
  Coverage   37.94%   37.94%           
=======================================
  Files          77       77           
  Lines        7784     7784           
  Branches     1667     1667           
=======================================
  Hits         2954     2954           
  Misses       4580     4580           
  Partials      250      250           
Flag Coverage Δ
unittests 37.93% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Collaborator

@thinkall thinkall left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @sharsha315 for the contribution!
Looks like you're using some tools to break the lines into length of 80. Could you share which tool is it? Maybe we can add it to the pre-commit tool and set the length to be 120 which is the same as the code length. And can we config the tool to not break a link?

autogen/agentchat/chat.py Outdated Show resolved Hide resolved
@gagb
Copy link
Collaborator

gagb commented Mar 25, 2024

Have you tried to build the website locally and see what the results look like in the API docs? See contrib docs for building website locally. You can take a screenshot of that much appreciated

Thank you for the PR!
To add to what Eric said. Also might wanna check if the doc string renders correctly in the IDE. Before/after screenshots would be nice. I think @jackgerrits recommended the google convention? It would be nice to update the description to say which convention you used.
Also closing #2048 would require edits to a lot of files.

Thanks for the feedback! I initially focused on fixing the two files mentioned in the issue. You said it would require edits to a lot of files, could you please clarify which files I'm allowed to change and which files allowed not to change. Also, I'll carefully look into your recommendation and update the description accordingly. Your guidance would be greatly appreciated, so that I can continue working on it. Thanks again.

I think we want to improve consistency across every files in library e.g., files in the main module autogen

I suggest that you try to get fixes to these two files reviewed but say that this PR only partially addresses the issue. Once multiple reviewers are happy, get this PR merged. Then create a new issue with a task list/plan and one-by-one make new PRs to improve a small set of files. This might help you contribute gradually without introducing massive merge conflicts (since module files might change). @jackgerrits and @ekzhu please add if I missed anything.

@ekzhu ekzhu added the documentation Improvements or additions to documentation label Mar 25, 2024
@sharsha315
Copy link
Contributor Author

Have you tried to build the website locally and see what the results look like in the API docs? See contrib docs for building website locally. You can take a screenshot of that much appreciated

Thank you for the PR!
To add to what Eric said. Also might wanna check if the doc string renders correctly in the IDE. Before/after screenshots would be nice. I think @jackgerrits recommended the google convention? It would be nice to update the description to say which convention you used.
Also closing #2048 would require edits to a lot of files.

Thanks for the feedback! I initially focused on fixing the two files mentioned in the issue. You said it would require edits to a lot of files, could you please clarify which files I'm allowed to change and which files allowed not to change. Also, I'll carefully look into your recommendation and update the description accordingly. Your guidance would be greatly appreciated, so that I can continue working on it. Thanks again.

I think we want to improve consistency across every files in library e.g., files in the main module autogen

I suggest that you try to get fixes to these two files reviewed but say that this PR only partially addresses the issue. Once multiple reviewers are happy, get this PR merged. Then create a new issue with a task list/plan and one-by-one make new PRs to improve a small set of files. This might help you contribute gradually without introducing massive merge conflicts (since module files might change). @jackgerrits and @ekzhu please add if I missed anything.

Thanks for clarifying the approach and providing guidance on how to proceed. I am on it.

* Add vision capability

* Configurate: description_prompt

* Print warning instead of raising issues for type

* Skip vision capability test if dependencies not installed

* Append "vision" to agent's system message when enabled VisionCapability

* GPT-4V notebook update with ConversableAgent

* Clean GPT-4V notebook

* Add vision capability test to workflow

* Lint import

* Update system message for vision capability

* Add a `custom_caption_func` to VisionCapability

* Add custom function example for vision capability

* Skip test Vision capability custom func

* GPT-4V notebook metadata to website

* Remove redundant files

* The custom caption function takes more inputs now

* Add a more complex example of custom caption func

* Remove trailing space

---------

Co-authored-by: Chi Wang <wang.chi@microsoft.com>
@sonichi sonichi requested a review from Josephrp April 1, 2024 04:34
@sharsha315
Copy link
Contributor Author

Hi @thinkall, just following up on my PR. Would appreciate any feedback or updates on its status. Thanks!

@sonichi sonichi enabled auto-merge April 1, 2024 13:57
Copy link
Contributor

@RohitRathore1 RohitRathore1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@RohitRathore1
Copy link
Contributor

RohitRathore1 commented Apr 2, 2024

@sharsha315 It would be nice if you can address this comment by @gagb

Also closing #2048 would require edits to a lot of files.

Because it will allow to close that issue if you can make the changes.

@sharsha315
Copy link
Contributor Author

sharsha315 commented Apr 2, 2024

@sharsha315 It would be nice if you can address this comment by @gagb

Also closing #2048 would require edits to a lot of files.

Because it will allow to close that issue if you can make the changes.

I was earlier suggested by @gagb , to first get reviewed this two files but as this partially addresses the issue, after multiple reviewers are happy, get this PR merged. Then create a new issue with a task list/plan and one-by-one make new PRs to improve a small set of files. This might help you contribute gradually without introducing massive merge conflicts (since module files might change).

Copy link
Member

@jackgerrits jackgerrits left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please update the description so that the relevant issue is not closed by this PR as there are other files to be edited too

@sharsha315
Copy link
Contributor Author

Could you please update the description so that the relevant issue is not closed by this PR as there are other files to be edited too

Hi @jackgerrits, I have updated the description, please review and provide feedback, any other information needs to be updated. Thank You.

@sonichi sonichi added this pull request to the merge queue Apr 3, 2024
Merged via the queue into microsoft:main with commit 2053dd9 Apr 3, 2024
30 of 72 checks passed
whiskyboy pushed a commit to whiskyboy/autogen that referenced this pull request Apr 17, 2024
* DOC FIX - Formatted Docstrings for the retrieve_user_proxy_agent.py and Added first single line for the class RetrieveUserProxyAgent.

* DOC FIX - Formatted Docstrings for  theinitiate_chats functiion of ChatResult class in  autogen/agentchat/chat.py

* Add vision capability (microsoft#2025)

* Add vision capability

* Configurate: description_prompt

* Print warning instead of raising issues for type

* Skip vision capability test if dependencies not installed

* Append "vision" to agent's system message when enabled VisionCapability

* GPT-4V notebook update with ConversableAgent

* Clean GPT-4V notebook

* Add vision capability test to workflow

* Lint import

* Update system message for vision capability

* Add a `custom_caption_func` to VisionCapability

* Add custom function example for vision capability

* Skip test Vision capability custom func

* GPT-4V notebook metadata to website

* Remove redundant files

* The custom caption function takes more inputs now

* Add a more complex example of custom caption func

* Remove trailing space

---------

Co-authored-by: Chi Wang <wang.chi@microsoft.com>

* Native tool call support for Mistral AI API and topic notebook. (microsoft#2135)

* Support for Mistral AI API and topic notebook.

* formatting

* formatting

* New conversational chess notebook using nested chats and tool use (microsoft#2137)

* add chess notebook

* update

* update

* Update notebook with figure

* Add example link

* redirect

* Clean up example format

* address gagan's comments

* update references

* fix links

* add webarena in samples (microsoft#2114)

* add webarena in samples/tools

* Update samples/tools/webarena/README.md

Co-authored-by: gagb <gagb@users.noreply.github.com>

* Update samples/tools/webarena/README.md

Co-authored-by: gagb <gagb@users.noreply.github.com>

* Update samples/tools/webarena/README.md

Co-authored-by: gagb <gagb@users.noreply.github.com>

* update installation instructions

* black formatting

* Update README.md

---------

Co-authored-by: gagb <gagb@users.noreply.github.com>
Co-authored-by: Eric Zhu <ekzhu@users.noreply.github.com>

* context to kwargs (microsoft#2064)

* context to kwargs

* add tag

* add test

* text to kwargs

---------

Co-authored-by: Eric Zhu <ekzhu@users.noreply.github.com>
Co-authored-by: Chi Wang <wang.chi@microsoft.com>

* Bump webpack-dev-middleware from 5.3.3 to 5.3.4 in /website (microsoft#2131)

Bumps [webpack-dev-middleware](https://github.com/webpack/webpack-dev-middleware) from 5.3.3 to 5.3.4.
- [Release notes](https://github.com/webpack/webpack-dev-middleware/releases)
- [Changelog](https://github.com/webpack/webpack-dev-middleware/blob/v5.3.4/CHANGELOG.md)
- [Commits](webpack/webpack-dev-middleware@v5.3.3...v5.3.4)

---
updated-dependencies:
- dependency-name: webpack-dev-middleware
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Eric Zhu <ekzhu@users.noreply.github.com>

* Parse Any HTML-esh Style Tags (microsoft#2046)

* tried implementing my own regex

* improves tests

* finally works

* removes prints

* fixed test

* adds start and end

* delete unused imports

* refactored to use new tool

* significantly improved algo

* tag content -> tag attr

* fix tests + adds new field

* return full match

* return remove start and end

* update docstrings

* update docstrings

* update docstrings

---------

Co-authored-by: Beibin Li <BeibinLi@users.noreply.github.com>
Co-authored-by: Chi Wang <wang.chi@microsoft.com>

* Integrate AgentOptimizer (microsoft#1767)

* draft agent optimizer

* refactor

* remove

* change openai config interface

* notebook

* update blog

* add test

* clean up

* redir

* update

* update interface

* change model name

* move to contrib

* Update autogen/agentchat/contrib/agent_optimizer.py

Co-authored-by: Jack Gerrits <jackgerrits@users.noreply.github.com>

---------

Co-authored-by: “skzhang1” <“shaokunzhang529@gmail.com”>
Co-authored-by: Beibin Li <BeibinLi@users.noreply.github.com>
Co-authored-by: Jieyu Zhang <jieyuz2@cs.washington.edu>
Co-authored-by: Jack Gerrits <jackgerrits@users.noreply.github.com>

* Introducing IOStream protocol and adding support for websockets (microsoft#1551)

* Introducing IOStream

* bug fixing

* polishing

* refactoring

* refactoring

* refactoring

* wip: async tests

* websockets added

* wip

* merge with main

* notebook added

* FastAPI example added

* wip

* merge

* getter/setter to iostream added

* website/blog/2024-03-03-AutoGen-Update/img/dalle_gpt4v.png: convert to Git LFS

* website/blog/2024-03-03-AutoGen-Update/img/gaia.png: convert to Git LFS

* website/blog/2024-03-03-AutoGen-Update/img/teach.png: convert to Git LFS

* add SSL support

* wip

* wip

* exception handling added to on_connect()

* refactoring: default iostream is being set in a context manager

* test fix

* polishing

* polishing

* polishing

* fixed bug with new thread

* polishing

* a bit of refactoring and docs added

* notebook added to docs

* type checking added to CI

* CI fix

* CI fix

* CI fix

* polishing

* obsolete todo comment removed

* fixed precommit error

---------

Co-authored-by: Eric Zhu <ekzhu@users.noreply.github.com>

* [CAP] [Feature] Get list of actors from directory service. (microsoft#2073)

* Search directory for list of actors using regex '.*' gets all actors

* docs changes

* pre-commit fixes

* Use ActorInfo from protobuf

* pre-commit

* Added zmq tests to work on removing sleeps

* minor refactor of zmq tests

* 1) Change DirSvr to user Broker.  2) Add req-router to broker 3) In ActorConnector use handshake and req/resp to remove sleep

* 1) Change DirSvr to user Broker.  2) Add req-router to broker 3) In ActorConnector use handshake and req/resp to remove sleep

* move socket creation to thread with recv

* move socket creation to thread with recv

* Better logging for DirectorySvc

* better logging for directory svc

* Use logging config

* Start removing sleeps

* pre-commit

* Cleanup monitor socket

* Mark cache as a protocol and update type hints to reflect (microsoft#2168)

* Mark cache as a protocl and update type hints to reflect

* int

* undo init change

	modified:   autogen/agentchat/chat.py

* fix(): fix word spelling errors (microsoft#2171)

* Implement User Defined Functions for Local CLI Executor (microsoft#2102)

* Implement user defined functions feature for local cli exec, add docs

* add tests, update docs

* fixes

* fix test

* add pandas test dep

* install test

* provide template as func

* formatting

* undo change

* address comments

* add test deps

* formatting

* test only in 1 env

* formatting

* remove test for local only

---------

Co-authored-by: Eric Zhu <ekzhu@users.noreply.github.com>

* simplify getting-started; update news (microsoft#2175)

* simplify getting-started; update news

* bug fix

* update (microsoft#2178)

Co-authored-by: AnonymousRepoSub <“shaokunzhang529@outlook.com” >

* Fix formatting of admonitions in udf docs (microsoft#2188)

* Fix iostream on new thread (microsoft#2181)

* fixed get_stream in new thread by introducing a global default

* fixed get_stream in new thread by introducing a global default

---------

Co-authored-by: Chi Wang <wang.chi@microsoft.com>

* Add link for rendering notebooks docs on website (microsoft#2191)

* Transform Messages Capability (microsoft#1923)

* wip

* Adds docstrings

* fixed spellings

* wip

* fixed errors

* better class names

* adds tests

* added tests to workflow

* improved token counting

* improved notebook

* improved token counting in test

* improved docstrings

* fix inconsistencies

* changed by mistake

* fixed docstring

* fixed details

* improves tests + adds openai contrib test

* fix spelling oai contrib test

* clearer docstrings

* remove repeated docstr

* improved notebook

* adds metadata to notebook

* Improve outline and description (microsoft#2125)

* better dir structure

* clip max tokens to allowed tokens

* more accurate comments/docstrs

* add deperecation warning

* fix front matter desc

* add deperecation warning notebook

* undo local notebook settings changes

* format notebook

* format workflow

---------

Co-authored-by: gagb <gagb@users.noreply.github.com>

* Bump express from 4.18.2 to 4.19.2 in /website (microsoft#2157)

Bumps [express](https://github.com/expressjs/express) from 4.18.2 to 4.19.2.
- [Release notes](https://github.com/expressjs/express/releases)
- [Changelog](https://github.com/expressjs/express/blob/master/History.md)
- [Commits](expressjs/express@4.18.2...4.19.2)

---
updated-dependencies:
- dependency-name: express
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* add clarity analytics (microsoft#2201)

* Docstring formatting fix: Standardize docstrings to adhere to Google style guide, ensuring consistency and clarity. and also fixed the broken link for autogen/agentchat/chat.py

* Docstring fix: Reformattted docstrings to adhere to Google style guide, nsuring consistency and clarity. For agentchat/contrib/retrieve_user_proxy_agent.py file

* Fixed Pre-Commit Error, Trailing spaces on agentchat/chat.py

* Fixed Pre-Commit Error, Trailing spaces on agentchat/chat.py

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: Li Jiang <bnujli@gmail.com>
Co-authored-by: Beibin Li <BeibinLi@users.noreply.github.com>
Co-authored-by: Chi Wang <wang.chi@microsoft.com>
Co-authored-by: Eric Zhu <ekzhu@users.noreply.github.com>
Co-authored-by: olgavrou <olgavrou@gmail.com>
Co-authored-by: gagb <gagb@users.noreply.github.com>
Co-authored-by: Qingyun Wu <qingyun0327@gmail.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Wael Karkoub <wael.karkoub96@gmail.com>
Co-authored-by: Shaokun Zhang <shaokunzhang529@gmail.com>
Co-authored-by: “skzhang1” <“shaokunzhang529@gmail.com”>
Co-authored-by: Jieyu Zhang <jieyuz2@cs.washington.edu>
Co-authored-by: Jack Gerrits <jackgerrits@users.noreply.github.com>
Co-authored-by: Davor Runje <davor@airt.ai>
Co-authored-by: Rajan <rajan.chari@yahoo.com>
Co-authored-by: calm <1191465097@qq.com>
Co-authored-by: AnonymousRepoSub <“shaokunzhang529@outlook.com” >
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.