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

added tests of remaining transformers #113

Merged
merged 4 commits into from
Feb 5, 2025
Merged

added tests of remaining transformers #113

merged 4 commits into from
Feb 5, 2025

Conversation

epinzur
Copy link
Collaborator

@epinzur epinzur commented Feb 5, 2025

Adds a new option to mark tests as extra. This requires the --runextras flag on pytest to run.

Adds tests for the transformers that were missing tests. Many require extras and are not tested against python 3.13.

Also simplifies the HierarchyLinkExtractor, now called the ParentTransformer.

This closes #83 and #59.

Copy link

github-actions bot commented Feb 5, 2025

Test Results

    8 files  ± 0    8 suites  ±0   2m 1s ⏱️ +26s
  328 tests + 5  328 ✅ + 5    0 💤 ±0  0 ❌ ±0 
1 312 runs  +20  780 ✅ +16  532 💤 +4  0 ❌ ±0 

Results for commit 583937b. ± Comparison against base commit c93576b.

This pull request removes 2 and adds 7 tests. Note that renamed tests count towards both.
packages.langchain-graph-retriever.tests.document_transformers.test_metadata_denormalizer ‑ test_revert_documents
packages.langchain-graph-retriever.tests.document_transformers.test_metadata_denormalizer ‑ test_transform_documents
packages.langchain-graph-retriever.tests.transformers.test_gliner ‑ test_transform_documents
packages.langchain-graph-retriever.tests.transformers.test_hierarchy ‑ test_transform_documents
packages.langchain-graph-retriever.tests.transformers.test_html_hyperlink ‑ test_transform_documents
packages.langchain-graph-retriever.tests.transformers.test_keybert ‑ test_transform_documents
packages.langchain-graph-retriever.tests.transformers.test_metadata_denormalizer ‑ test_revert_documents
packages.langchain-graph-retriever.tests.transformers.test_metadata_denormalizer ‑ test_transform_documents
packages.langchain-graph-retriever.tests.transformers.test_spacy ‑ test_transform_documents

♻️ This comment has been updated with latest results.

@epinzur epinzur force-pushed the update_transformers branch from fc235ba to 94da13b Compare February 5, 2025 16:55
from typing_extensions import override


class ParentTransformer(BaseDocumentTransformer):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nice. I think it's useful to have this transformer (especially for LangFlow, etc.) but this could be a good thing to have an example of doing too. Specifically, if the user has already written code to put the path in the metadata... it wouldn't be too hard to just write the parent themselves.

We should definitely have some examples showing "here is how to set the metadata for X" rather than relying on hiding it all in transformers. Specifically, that enables the user to encode their own structured metadata.

This would be a good example to use, since it demonstrates "setting something like parent in the metadata enables different kinds of navigation via the edges".

@epinzur epinzur force-pushed the update_transformers branch from 26aafb9 to 4e2422e Compare February 5, 2025 18:15
@epinzur epinzur force-pushed the update_transformers branch from 4e2422e to 017cf4e Compare February 5, 2025 18:18
@coveralls
Copy link

coveralls commented Feb 5, 2025

Pull Request Test Coverage Report for Build 13164213708

Details

  • 43 of 47 (91.49%) changed or added relevant lines in 5 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+10.3%) to 92.916%

Changes Missing Coverage Covered Lines Changed/Added Lines %
packages/langchain-graph-retriever/src/langchain_graph_retriever/transformers/keybert.py 6 7 85.71%
packages/langchain-graph-retriever/src/langchain_graph_retriever/transformers/html_hyperlink.py 9 12 75.0%
Totals Coverage Status
Change from base Build 13163802757: 10.3%
Covered Lines: 1468
Relevant Lines: 1554

💛 - Coveralls

@epinzur epinzur force-pushed the update_transformers branch from 5313962 to 583937b Compare February 5, 2025 18:34
@epinzur epinzur merged commit e9ae582 into main Feb 5, 2025
11 checks passed
@epinzur epinzur deleted the update_transformers branch February 5, 2025 18:41
@epinzur epinzur mentioned this pull request Feb 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add CI test environment with no extras installed
3 participants