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.

Sign up for GitHub

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

Jump to bottom

Add docstring linting to core, langchain, partner packages #23188

Closed
1 task done
baskaryan opened this issue Jun 19, 2024 · 3 comments
Closed
1 task done

Add docstring linting to core, langchain, partner packages #23188

baskaryan opened this issue Jun 19, 2024 · 3 comments
Labels
🤖:docs Changes to documentation and examples, like .md, .rst, .ipynb files. Changes to the docs/ folder help wanted Good issue for contributors

Comments

@baskaryan
Copy link
Collaborator

baskaryan commented Jun 19, 2024 •
edited
Loading

Privileged issue

  • I am a LangChain maintainer, or was asked directly by a LangChain maintainer to create an issue here.

Issue Content

We want to add docstring linting to langchain-core, langchain, langchain-text-splitters, and partner packages. This requires adding this to each pacakages pyproject.toml

[tool.ruff.lint]
select = [
  ...
  "D",    # pydocstyle
]

[tool.ruff.lint.pydocstyle]
convention = "google"

[tool.ruff.per-file-ignores]
"tests/**" = ["D"]  # ignore docstring checks for tests

this will likely cause a number of new linting errors which then need to be fixed. there should be a separate pr for each package. here's a reference for langchain-openai (linting errors have not yet been fixed) #23187
@baskaryan baskaryan added the help wanted Good issue for contributors label Jun 19, 2024
@dosubot dosubot bot added Ɑ: core Related to langchain-core Ɑ: text splitters Related to text splitters package 🤖:docs Changes to documentation and examples, like .md, .rst, .ipynb files. Changes to the docs/ folder labels Jun 19, 2024
@baskaryan baskaryan removed Ɑ: text splitters Related to text splitters package Ɑ: core Related to langchain-core labels Jun 19, 2024
@spazm spazm mentioned this issue Jun 20, 2024
Closed
@Zizo-Vi Zizo-Vi mentioned this issue Jun 21, 2024
Merged
@Zizo-Vi
Copy link
Contributor

Zizo-Vi commented Jun 21, 2024

@baskaryan please review this pr：#23249

eyurtsev pushed a commit that referenced this issue Jun 21, 2024
@Zizo-Vi
partners: add lint docstrings for chroma module (#23249)
f9aea3d 
Description: add lint docstrings for chroma module
Issue: the issue #23188 @baskaryan

test:  ruff check passed.


![image](https://github.com/langchain-ai/langchain/assets/76683249/5e168a0c-32d0-464f-8ddb-110233918019)

---------

Co-authored-by: gongwn1 <[email protected]>
This was referenced Jun 22, 2024
Merged
Merged
ccurme pushed a commit that referenced this issue Jun 24, 2024
@Zizo-Vi
partners: add lint docstrings for azure-dynamic-sessions/together mod…
af620db 
…ules (#23303)

Description: add lint docstrings for azure-dynamic-sessions/together
modules
Issue: #23188 @baskaryan

test: ruff check passed.
<img width="782" alt="image"
src="https://github.com/langchain-ai/langchain/assets/76683249/bf11783d-65b3-4e56-a563-255eae89a3e4">

---------

Co-authored-by: gongwn1 <[email protected]>
Josephasafg pushed a commit to joshc-ai21/langchain that referenced this issue Jun 25, 2024
@Zizo-Vi @asafgardin
partners: add lint docstrings for azure-dynamic-sessions/together mod…
7ae0fbc 
…ules (langchain-ai#23303)

Description: add lint docstrings for azure-dynamic-sessions/together
modules
Issue: langchain-ai#23188 @baskaryan

test: ruff check passed.
<img width="782" alt="image"
src="https://github.com/langchain-ai/langchain/assets/76683249/bf11783d-65b3-4e56-a563-255eae89a3e4">

---------

Co-authored-by: gongwn1 <[email protected]>
@Zizo-Vi Zizo-Vi mentioned this issue Jul 3, 2024
Closed
mandos21 pushed a commit to mandos21/langchain that referenced this issue Jul 8, 2024
@Zizo-Vi @mandos21
partners: add lint docstrings for azure-dynamic-sessions/together mod…
531c8ed 
…ules (langchain-ai#23303)

Description: add lint docstrings for azure-dynamic-sessions/together
modules
Issue: langchain-ai#23188 @baskaryan

test: ruff check passed.
<img width="782" alt="image"
src="https://github.com/langchain-ai/langchain/assets/76683249/bf11783d-65b3-4e56-a563-255eae89a3e4">

---------

Co-authored-by: gongwn1 <[email protected]>
@dosubot dosubot bot added the stale Issue has not had recent activity or appears to be solved. Stale issues will be automatically closed label Sep 20, 2024
@dosubot dosubot bot closed this as not planned Won't fix, can't repro, duplicate, stale Sep 27, 2024
@dosubot dosubot bot removed the stale Issue has not had recent activity or appears to be solved. Stale issues will be automatically closed label Sep 27, 2024
@JiaranI JiaranI mentioned this issue Oct 28, 2024
Merged
@JiaranI
Copy link
Contributor

JiaranI commented Oct 28, 2024

@baskaryan, could you please review this PR: #27686?
As I'm new here, I would appreciate any feedback or suggestions if there's anything I've missed. Thank you!

efriis added a commit that referenced this issue Oct 31, 2024
@JiaranI @efriis
ollama: add pydocstyle linting for ollama (#27686)
3952ee3 
Description: add lint docstrings for ollama module
Issue: the issue #23188
@baskaryan

test: ruff check passed.
<img width="311" alt="e94c68ffa93dd518297a95a93de5217"
src="https://github.com/user-attachments/assets/e96bf721-e0e3-44de-a50e-206603de398e">

Co-authored-by: Erick Friis <[email protected]>
@dangiankit dangiankit mentioned this issue Nov 15, 2024
Merged
@dangiankit
Copy link
Contributor

dangiankit commented Nov 15, 2024 •
edited
Loading

Alternatively, for the text-splitters package in #28127, instead of the top-level config for ruff:

[tool.ruff.per-file-ignores]
"tests/**" = ["D"]  # ignore docstring checks for tests

the below mentioned nested config worked.

 [tool.ruff.lint.per-file-ignores]
 "tests/**" = ["D"]  # ignore docstring checks for tests

efriis added a commit that referenced this issue Dec 9, 2024
@dangiankit @efriis
text-splitters: add pydocstyle linting (#28127)
90f162e 
As seen in #23188, turned on Google-style docstrings by enabling
`pydocstyle` linting in the `text-splitters` package. Each resulting
linting error was addressed differently: ignored, resolved, suppressed,
and missing docstrings were added.

Fixes one of the checklist items from #25154, similar to #25939 in
`core` package. Ran `make format`, `make lint` and `make test` from the
root of the package `text-splitters` to ensure no issues were found.

---------

Co-authored-by: Erick Friis <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
🤖:docs Changes to documentation and examples, like .md, .rst, .ipynb files. Changes to the docs/ folder help wanted Good issue for contributors
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@dangiankit @baskaryan @Zizo-Vi @JiaranI