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: Add how-to guide for SQLDatabaseLoader #27696

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

amotl
Copy link
Contributor

@amotl amotl commented Oct 28, 2024

Dear Eugene,

coming from GH-16246/GH-18281, this patch adds missing documentation that has still been included in crate-workbench#1. We wanted to break it out of that patch, in order to further minimise its size before submitting it.

With kind regards,
Andreas.

NB: Can I also humbly ask you to review/exercise this for validity, @simonprickett or @kneth? I think the demonstrated code should work with recent vanilla versions of LangChain, with all of SQLite, PostgreSQL, and CrateDB.

@dosubot dosubot bot added the size:XL This PR changes 500-999 lines, ignoring generated files. label Oct 28, 2024
Copy link

vercel bot commented Oct 28, 2024

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

Name Status Preview Comments Updated (UTC)
langchain ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 28, 2024 11:23pm

@dosubot dosubot bot added community Related to langchain-community Ɑ: doc loader Related to document loader module (not documentation) 🤖:docs Changes to documentation and examples, like .md, .rst, .ipynb files. Changes to the docs/ folder labels Oct 28, 2024
Copy link
Contributor Author

@amotl amotl left a comment

Choose a reason for hiding this comment

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

Submitting just a few comments from self-review.

"cell_type": "markdown",
"metadata": {},
"source": [
"# SQL Database\n",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Suggested change
"# SQL Database\n",
"# SQLDatabaseLoader\n",

Comment on lines +15 to +16
"For talking to the database, the document loader uses the [SQLDatabase]\n",
"utility from the LangChain integration toolkit.\n",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Suggested change
"For talking to the database, the document loader uses the [SQLDatabase]\n",
"utility from the LangChain integration toolkit.\n",
"For talking to the database, the document loader uses the [SQLDatabase]\n",
"utility.\n",

"\n",
"[SQLAlchemy]: https://www.sqlalchemy.org/\n",
"[SQLAlchemy dialects]: https://docs.sqlalchemy.org/en/latest/dialects/\n",
"[SQLDatabase]: https://python.langchain.com/docs/integrations/toolkits/sql_database\n",
Copy link
Contributor Author

@amotl amotl Oct 29, 2024

Choose a reason for hiding this comment

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

Linking to the SQLDatabase Toolkit was probably wrong, so let's better link to the Utility instead?

Suggested change
"[SQLDatabase]: https://python.langchain.com/docs/integrations/toolkits/sql_database\n",
"[SQLDatabase]: https://python.langchain.com/api_reference/community/utilities/langchain_community.utilities.sql_database.SQLDatabase.html\n",

Is there a better piece of dedicated documentation about this component elsewhere in the documentation tree, i.e. are you advising to use a different target link here?

@eyurtsev eyurtsev self-assigned this Oct 29, 2024
@eyurtsev eyurtsev changed the title community: Add documentation about SQLDatabaseLoader docs: Add how-to guide for SQLDatabaseLoader Oct 29, 2024
@eyurtsev
Copy link
Collaborator

Hi @amotl, thank you for taking the initiative to improve the how-to guides!

We don't have good guidelines yet for the how-to guides, but if you can browse through a few (https://python.langchain.com/docs/how_to/), you may get a bit of a sense of the format:

  1. Use a notebook format only here (remove the mdx file).
  2. Add a link to the notebook from the how-to guide index in an appropriate place
  3. The title should be phrased as a how-to question (e.g., "How to ...")
  4. If you can link to conceptual prerequisites (e.g., https://python.langchain.com/docs/how_to/passthrough/). Potentially OK to link to any SQL resources that make sense for folks to know about
  5. Don't comment out the package install (maybe simply skip it if you already have the packages installed)
  6. Review https://diataxis.fr/how-to-guides/ if you're not familiar with diataxis

@amotl
Copy link
Contributor Author

amotl commented Oct 29, 2024

Thank you for your guidance, Eugene. I will rework the patch according to your suggestions, and come back afterwards. The same guidelines will probably also apply to GH-27713.

Both pieces of documentation are coming from initially being conceived around LangChain <0.1 times, where blueprints from other how-to documents have not been so elaborate yet.

Copy link

@kneth kneth left a comment

Choose a reason for hiding this comment

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

LGTM

I would appreciate additional reviewers

@amotl
Copy link
Contributor Author

amotl commented Nov 1, 2024

@kneth: The ingredients of this patch need further love, as outlined by @eyurtsev. That's why we toggled it into draft mode again. Relevant patches are welcome.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community Related to langchain-community Ɑ: doc loader Related to document loader module (not documentation) 🤖:docs Changes to documentation and examples, like .md, .rst, .ipynb files. Changes to the docs/ folder size:XL This PR changes 500-999 lines, ignoring generated files.
Projects
Status: In review
Development

Successfully merging this pull request may close these issues.

3 participants