Skip to content

Refactors index functions #269

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

Merged
merged 33 commits into from
Feb 14, 2025
Merged

Conversation

alexthomas93
Copy link
Contributor

@alexthomas93 alexthomas93 commented Feb 10, 2025

Description

This PR refactors the code for interacting with vector and hybrid indexes. Key changes include:

  • Refactors get_search_query to make it compatible with LangChain Neo4j.
  • Adds retrieve_fulltext_index_info utility function: Retrieves metadata about existing full-text indexes.
  • Adds retrieve_vector_index_info utility function: Retrieves metadata about existing vector indexes.
  • Adds the effective_search_ratio parameter for vector and hybrid search.

Type of Change

  • New feature
  • Bug fix
  • Breaking change
  • Documentation update
  • Project configuration change

Complexity

Complexity: Medium

How Has This Been Tested?

  • Unit tests
  • E2E tests
  • Manual tests

Checklist

The following requirements should have been met (depending on the changes in the branch):

  • Documentation has been updated
  • Unit tests have been updated
  • E2E tests have been updated
  • Examples have been updated
  • New files have copyright header
  • CLA (https://neo4j.com/developer/cla/) has been signed
  • CHANGELOG.md updated if appropriate

@alexthomas93 alexthomas93 requested a review from a team as a code owner February 10, 2025 21:02
@alexthomas93 alexthomas93 marked this pull request as draft February 11, 2025 16:09
@alexthomas93 alexthomas93 marked this pull request as ready for review February 11, 2025 16:59
@willtai
Copy link
Contributor

willtai commented Feb 12, 2025

Deprecates the id property returned by all retrievers.

This will be a breaking change. Why do we make this change in this PR?

@alexthomas93
Copy link
Contributor Author

Deprecates the id property returned by all retrievers.

This will be a breaking change. Why do we make this change in this PR?

We don't necessarily. Happy to remove it and make the change in another PR. It just felt like given I was already refactoring these functions now might be a good time to do it. I think there have also been a few releases now since the warning this property would be removed was added.

@alexthomas93
Copy link
Contributor Author

@CodiumAI-Agent /update_changelog

@CodiumAI-Agent
Copy link

Changelog updates: 🔄

2025-02-12 *

Added

  • Utility functions to retrieve metadata for vector and full-text indexes.
  • Support for effective_search_ratio parameter in vector and hybrid searches.

Changed

  • Refactored index-related functions for improved compatibility and functionality.
  • Deprecated the id property in retriever results.

to commit the new content to the CHANGELOG.md file, please type:
'/update_changelog --pr_update_changelog.push_changelog_changes=true'

@willtai
Copy link
Contributor

willtai commented Feb 13, 2025

We don't necessarily. Happy to remove it and make the change in another PR. It just felt like given I was already refactoring these functions now might be a good time to do it. I think there have also been a few releases now since the warning this property would be removed was added.

Don't think we need it for this PR. We can remove it in a separate PR, wdyt?

@CodiumAI-Agent
Copy link

Changelog updates: 🔄

2025-02-13

Added

  • Utility functions to retrieve metadata for vector and full-text indexes.
  • Support for effective_search_ratio parameter in vector and hybrid searches.

Changed

  • Refactored index-related functions for improved compatibility and functionality.
  • Deprecated the id property in retriever results.

to commit the new content to the CHANGELOG.md file, please type:
'/update_changelog --pr_update_changelog.push_changelog_changes=true'

@alexthomas93
Copy link
Contributor Author

alexthomas93 commented Feb 13, 2025

We don't necessarily. Happy to remove it and make the change in another PR. It just felt like given I was already refactoring these functions now might be a good time to do it. I think there have also been a few releases now since the warning this property would be removed was added.

Don't think we need it for this PR. We can remove it in a separate PR, wdyt?

Makes sense. I'll add something to the backlog to remind us to deprecate it at some point in the future. Have re-added it in e773de4. PR should no longer have any breaking changes in it.

Copy link
Contributor

@willtai willtai left a comment

Choose a reason for hiding this comment

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

☄️

@alexthomas93 alexthomas93 merged commit a414ca6 into neo4j:main Feb 14, 2025
7 checks passed
@alexthomas93 alexthomas93 deleted the refactor/index branch February 14, 2025 11:27
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.

4 participants