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

python312Packages.chromadb: 0.5.7 -> 0.5.11 #345372

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

fabaff
Copy link
Member

@fabaff fabaff commented Sep 29, 2024

Diff: chroma-core/chroma@refs/tags/0.5.7...0.5.11

Changelog: https://github.com/chroma-core/chroma/releases/tag/0.5.11

Description of changes

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 24.11 Release Notes (or backporting 23.11 and 24.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@fabaff
Copy link
Member Author

fabaff commented Sep 29, 2024

Result of nixpkgs-review pr 345372 run on x86_64-linux 1

4 packages marked as broken and skipped:
  • python311Packages.llama-index
  • python311Packages.llama-index.dist
  • python311Packages.private-gpt
  • python311Packages.private-gpt.dist
6 packages failed to build:
  • open-webui
  • open-webui.dist
  • python311Packages.langchain-chroma
  • python311Packages.langchain-chroma.dist
  • python312Packages.langchain-chroma
  • python312Packages.langchain-chroma.dist
8 packages built:
  • python311Packages.chromadb
  • python311Packages.chromadb.dist
  • python311Packages.llama-index-cli
  • python311Packages.llama-index-cli.dist
  • python311Packages.llama-index-vector-stores-chroma
  • python311Packages.llama-index-vector-stores-chroma.dist
  • python312Packages.chromadb
  • python312Packages.chromadb.dist

@fabaff
Copy link
Member Author

fabaff commented Sep 29, 2024

Looks like langchain-chroma needs an update too.

@sarahec
Copy link
Contributor

sarahec commented Oct 1, 2024

I'll push out the needed langchain updates this afternoon.

You also have a flaky test at chromadb/test/proto/test_utils.py::test_retry_interceptor

@sarahec
Copy link
Contributor

sarahec commented Oct 1, 2024

langchain-chroma is current at 0.1.4 (PR #341921) but contains a testing error upstream.

Merging #345803 will let langchain-chroma pass all tests and unblock your PR.

@fabaff
Copy link
Member Author

fabaff commented Oct 2, 2024

Uff, more failing tests

error: builder for '/nix/store/fvad1rlcb3sxsx98l798ni2jjrazkshz-python3.11-langchain-chroma-0.1.4.drv' failed with exit code 1;
       last 10 log lines:
       > tests/integration_tests/test_vectorstores.py:257: ValueError
       > ============================= slowest 5 durations ==============================
       > 0.67s call     tests/integration_tests/test_vectorstores.py::test_chroma_with_persistence
       > 0.11s call     tests/integration_tests/test_vectorstores.py::test_chroma
       > 0.01s call     tests/integration_tests/test_vectorstores.py::test_chroma_update_document
       > 0.01s call     tests/integration_tests/test_vectorstores.py::test_chroma_search_filter
       > 0.01s call     tests/integration_tests/test_vectorstores.py::test_chroma_async
       > =========================== short test summary info ============================
       > FAILED tests/integration_tests/test_vectorstores.py::test_chroma_update_document - ValueError: The truth value of an array with more than one element is ambig...
       > =================== 1 failed, 26 passed, 3 skipped in 1.55s ====================

@sarahec
Copy link
Contributor

sarahec commented Oct 2, 2024

Uff, more failing tests

This error is a side-effect of the async test not being called properly (fixed by the PR).

Well, nuts. Uff, as you say. Fixing the async test made this work yesterday but it's failing again today. (It also worked in the nixpkgs-review last night). I will need to dig more deeply.

@sarahec
Copy link
Contributor

sarahec commented Oct 2, 2024

Turns out that the langchain repo's integration tests fail the same way outside nixos:

        # Assert that the updated document is returned by the search
        assert output == [Document(page_content=updated_content, metadata={"page": "0"})]
    
>       assert new_embedding == embedding.embed_documents([updated_content])[0]
E       ValueError: The truth value of an array with more than one element is ambiguous. Use a.any() or a.all()

tests/integration_tests/test_vectorstores.py:257: ValueError
======================================================================= warnings summary ========================================================================
tests/integration_tests/test_vectorstores.py::test_chroma_async
  /Users/seclark/development/langchain/.devenv/state/venv/lib/python3.11/site-packages/_pytest/python.py:148: PytestUnhandledCoroutineWarning: async def functions are not natively supported and have been skipped.
  You need to install a suitable plugin for your async framework, for example:
    - anyio
    - pytest-asyncio
    - pytest-tornasync
    - pytest-trio
    - pytest-twisted
    warnings.warn(PytestUnhandledCoroutineWarning(msg.format(nodeid)))

-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html
====================================================================== slowest 5 durations ======================================================================
0.07s call     tests/integration_tests/test_vectorstores.py::test_chroma
0.03s call     tests/integration_tests/test_vectorstores.py::test_chroma_with_persistence
0.01s call     tests/integration_tests/test_vectorstores.py::test_chroma_update_document
0.01s call     tests/integration_tests/test_vectorstores.py::test_reset_collection
0.01s call     tests/integration_tests/test_vectorstores.py::test_chroma_search_filter_with_scores
==================================================================== short test summary info ====================================================================
FAILED tests/integration_tests/test_vectorstores.py::test_chroma_update_document - ValueError: The truth value of an array with more than one element is ambiguous. Use a.any() or a.all()
====================================================== 1 failed, 24 passed, 4 skipped, 1 warning in 0.78s =======================================================

I'm willing to call test_chroma_update_document a bad test and plan on disabling it.

@sarahec
Copy link
Contributor

sarahec commented Oct 3, 2024

Result of nixpkgs-review pr 345372 run on aarch64-darwin 1

10 packages marked as broken and skipped:
  • open-webui
  • open-webui.dist
  • python311Packages.llama-index
  • python311Packages.llama-index-cli
  • python311Packages.llama-index-cli.dist
  • python311Packages.llama-index-vector-stores-chroma
  • python311Packages.llama-index-vector-stores-chroma.dist
  • python311Packages.llama-index.dist
  • python311Packages.private-gpt
  • python311Packages.private-gpt.dist
8 packages built:
  • python311Packages.chromadb
  • python311Packages.chromadb.dist
  • python311Packages.langchain-chroma
  • python311Packages.langchain-chroma.dist
  • python312Packages.chromadb
  • python312Packages.chromadb.dist
  • python312Packages.langchain-chroma
  • python312Packages.langchain-chroma.dist

@sarahec
Copy link
Contributor

sarahec commented Oct 3, 2024

Looks like you are good to merge. When you mark it ready to merge, I'll give the LGTM.

Copy link
Contributor

@sarahec sarahec left a comment

Choose a reason for hiding this comment

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

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants