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

docs: integrations reference updates 3 #24796

Open
wants to merge 13 commits into
base: master
Choose a base branch
from

Conversation

leo-gan
Copy link
Collaborator

@leo-gan leo-gan commented Jul 29, 2024

Added missed provider pages and links. Fixed inconsistent formatting.

Copy link

vercel bot commented Jul 29, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
langchain ❌ Failed (Inspect) Sep 25, 2024 6:57pm

@leo-gan leo-gan marked this pull request as ready for review July 29, 2024 22:44
@dosubot dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. 🤖:docs Changes to documentation and examples, like .md, .rst, .ipynb files. Changes to the docs/ folder labels Jul 29, 2024
@leo-gan leo-gan requested a review from ccurme August 1, 2024 21:06
@leo-gan leo-gan changed the title docs: integration update references docs: integrations reference updates Aug 9, 2024
@leo-gan leo-gan requested review from eyurtsev and removed request for ccurme August 12, 2024 20:36
@leo-gan leo-gan force-pushed the docs-integrations-missed-references-fix-3 branch from 239ef74 to 30139c4 Compare August 13, 2024 17:22
@leo-gan
Copy link
Collaborator Author

leo-gan commented Aug 13, 2024

@efriis Erick, could you, please, make the lint error more informative? Thanks!
With the current error, I cannot localize what is wrong.
The error is:

Traceback (most recent call last):
File "/home/runner/work/langchain/langchain/docs/scripts/check_templates.py", line 95, in
Checking llms page /home/runner/work/langchain/langchain/docs/docs/integrations/llms/rwkv.mdx
main(*sys.argv[1:])
File "/home/runner/work/langchain/langchain/docs/scripts/check_templates.py", line 89, in main
check_header_order(path)
File "/home/runner/work/langchain/langchain/docs/scripts/check_templates.py", line 80, in check_header_order
raise ValueError(
ValueError: Document /home/runner/work/langchain/langchain/docs/docs/integrations/llms/rwkv.mdx does not match the expected header order. Please see #24803 for instructions on how to correctly format a llms integration page.

@leo-gan leo-gan force-pushed the docs-integrations-missed-references-fix-3 branch from 5d45f6c to e696328 Compare August 14, 2024 15:55
@leo-gan leo-gan changed the title docs: integrations reference updates docs: integrations reference updates 3 Aug 15, 2024
@leo-gan leo-gan force-pushed the docs-integrations-missed-references-fix-3 branch from 60863da to 6b6965f Compare August 20, 2024 01:01
@leo-gan leo-gan force-pushed the docs-integrations-missed-references-fix-3 branch from 6b6965f to f43104e Compare August 26, 2024 21:29
@leo-gan
Copy link
Collaborator Author

leo-gan commented Aug 26, 2024

@hwchase17 It is tempting to enforce a template to new examples. But it doesn't work if the template is not tested on the old examples. I found no old LLM example that fits this new template. How about "eating dog food" before enforcing?

@efriis
Copy link
Member

efriis commented Aug 26, 2024

Hey @leo-gan ! We're only enforcing this for newly contributed docs. The linked issue in the error message outlines exactly how to bootstrap this document and the template to match

Let me know if that doesn't make sense

@leo-gan
Copy link
Collaborator Author

leo-gan commented Aug 26, 2024

Hey @leo-gan ! We're only enforcing this for newly contributed docs. The linked issue in the error message outlines exactly how to bootstrap this document and the template to match

Let me know if that doesn't make sense

@efriis Erick,

  1. a template is the template, if it is repeated in most documents. Only then does it start to help the user (because the user is working now with a consistent format). If we have 90% of examples in free format, the template is useless. Let's apply the template on a significant portion of documents and only then enforce this template.
  2. I'm sure that after applying this template to 5-10 documents, this template will change, and the template enforcement code will change. Enforcing this template now is premature.
  3. The existing LLM template proposes a heading that, if applied to all documents, requires significant changes in existing documents (not only the head renaming). Some examples are simple, some are complex and detailed. Simple examples are better with the subset of the template headers, complex examples are better with supersets. The strict heading enforcement is too limiting.

@efriis
Copy link
Member

efriis commented Aug 27, 2024

Appreciate the feedback. Which heading is creating the most issues for you?

Updated the error message to be more clear which headings are missing in #25782

@leo-gan
Copy link
Collaborator Author

leo-gan commented Aug 27, 2024

@efriis "Setup" and "Installation" from the template is usually a single section "Setup and installation" or "Installation and Setup". Both should be OK, because it is clear for the reader.
The "Overview" section is usually a text after page Title and the "Installation/Setup" section. No special header
The "Instantiation" section is usually one-liner and a part of any other sections with code, but I like when it is in a separate section. It emphasizes the usage of this specific class/function.
The "Invocation" is usually the "Example" or several sections with examples. It is very flexible. See the example. If we enforce heading, we should completely refactor this example, not just rename titles and move pieces of the document.
Here is another example.
Here are the LLM examples: 1, , 2

To summarize:

  • General description [under the page title] can be enforced
  • generalizing Installation and Setup [under one section] - can be enforced.
  • enforcing anything else requires a total refactoring of all examples, which is not feasible and brings too little to usability/readability/comprehension

@efriis
Copy link
Member

efriis commented Aug 27, 2024

got it, and thank you for writing up. Let's stick with the templates for now (and fix this PR if it's desired), as we'd like to standardize the format a bit more than you outlined.

@leo-gan
Copy link
Collaborator Author

leo-gan commented Aug 27, 2024

Righ now the error:
ValueError:

Document /home/runner/work/langchain/langchain/docs/docs/integrations/llms/rwkv.mdx is missing headers:

  • Integration details

  • Credentials

  • Installation

  • Chaining

  • API reference

  • do we need to enforce H3+ sections? (### )
  • We use LLMs in many components not only in chains, so why this "## Chaining" section?
  • "## API reference" We have the APR Reference automatically in the "Instantiation" section. For example,
    API Reference: OpenAI
    Manually added link is not maintainable and confuses users (Why do we have two API Reference links? Do they equal? What is the difference?)

docs/docs/integrations/providers/ainetwork.mdx Outdated Show resolved Hide resolved
docs/docs/integrations/providers/amadeus.mdx Outdated Show resolved Hide resolved
docs/docs/integrations/providers/slack.mdx Outdated Show resolved Hide resolved
@efriis
Copy link
Member

efriis commented Sep 19, 2024

as a heads up, building the docs locally will yield the same broken link errors vercel does

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🤖:docs Changes to documentation and examples, like .md, .rst, .ipynb files. Changes to the docs/ folder size:L This PR changes 100-499 lines, ignoring generated files.
Projects
Status: In review
Development

Successfully merging this pull request may close these issues.

2 participants