From a669abf16b3ac3dcf10629936d3c58411469bb3c Mon Sep 17 00:00:00 2001 From: Eugene Yurtsev Date: Fri, 26 May 2023 10:18:11 -0400 Subject: [PATCH] Update CONTRIBUTION guidelines and PR Template (#5140) # Update contribution guidelines and PR template This PR updates the contribution guidelines to include more information on how to handle optional dependencies. The PR template is updated to include a link to the contribution guidelines document. --- .github/CONTRIBUTING.md | 41 +++++++++++++++++++++++++++++ .github/PULL_REQUEST_TEMPLATE.md | 44 ++++++++++++++++++++------------ 2 files changed, 68 insertions(+), 17 deletions(-) diff --git a/.github/CONTRIBUTING.md b/.github/CONTRIBUTING.md index 4eb88738..f878c9ea 100644 --- a/.github/CONTRIBUTING.md +++ b/.github/CONTRIBUTING.md @@ -115,8 +115,37 @@ To get a report of current coverage, run the following: make coverage ``` +### Working with Optional Dependencies + +Langchain relies heavily on optional dependencies to keep the Langchain package lightweight. + +If you're adding a new dependency to Langchain, assume that it will be an optional dependency, and +that most users won't have it installed. + +Users that do not have the dependency installed should be able to **import** your code without +any side effects (no warnings, no errors, no exceptions). + +To introduce the dependency to the pyproject.toml file correctly, please do the following: + +1. Add the dependency to the main group as an optional dependency + ```bash + poetry add --optional [package_name] + ``` +2. Open pyproject.toml and add the dependency to the `extended_testing` extra +3. Relock the poetry file to update the extra. + ```bash + poetry lock --no-update + ``` +4. Add a unit test that the very least attempts to import the new code. Ideally the unit +test makes use of lightweight fixtures to test the logic of the code. +5. Please use the `@pytest.mark.requires(package_name)` decorator for any tests that require the dependency. + ### Testing +See section about optional dependencies. + +#### Unit Tests + Unit tests cover modular logic that does not require calls to outside APIs. To run unit tests: @@ -133,8 +162,20 @@ make docker_tests If you add new logic, please add a unit test. + + +#### Integration Tests + Integration tests cover logic that requires making calls to outside APIs (often integration with other services). +**warning** Almost no tests should be integration tests. + + Tests that require making network connections make it difficult for other + developers to test the code. + + Instead favor relying on `responses` library and/or mock.patch to mock + requests using small fixtures. + To run integration tests: ```bash diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index fbc13b1b..bb1ee9af 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -1,7 +1,7 @@ # Your PR Title (What it does) + ## Who can review? @@ -22,25 +32,25 @@ Community members can review the PR once tests pass. Tag maintainers/contributor