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

feat: updated index docs #502

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat: updated index docs #502

wants to merge 1 commit into from

Conversation

Joshua-Briggs
Copy link
Member

@Joshua-Briggs Joshua-Briggs commented Jan 7, 2025

User description

note: async is in another PR,
sync has already been done,
pdrant is displaying issues due to libraries, I'll leave this one in a todo for later


PR Type

Documentation, Enhancement


Description

  • Updated local.ipynb and pinecone.ipynb notebooks to reflect new API changes.

  • Replaced RouteLayer with SemanticRouter for enhanced functionality.

  • Updated dependencies and configurations for compatibility.

  • Improved logging and error handling in example outputs.


Changes walkthrough 📝

Relevant files
Documentation
local.ipynb
Update local index example with SemanticRouter                     

docs/indexes/local.ipynb

  • Replaced RouteLayer with SemanticRouter for local index.
  • Updated execution outputs and logging messages.
  • Adjusted configurations for LocalIndex with auto_sync.
  • Removed outdated warnings and streamlined code cells.
  • +33/-32 
    pinecone.ipynb
    Update Pinecone index example with SemanticRouter               

    docs/indexes/pinecone.ipynb

  • Replaced RouteLayer with SemanticRouter for Pinecone index.
  • Updated dependency version for semantic-router[pinecone].
  • Enhanced logging and error outputs for Pinecone operations.
  • Adjusted configurations for PineconeIndex with auto_sync.
  • +86/-71 

    💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

    note: async is in another PR,
    sync has already been done,
    pdrant is displaying issues due to libraries, I'll leave this one in a PR for later
    Copy link

    github-actions bot commented Jan 7, 2025

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪
    🧪 No relevant tests
    🔒 Security concerns

    API Key Handling:
    The use of getpass for OPENAI_API_KEY and PINECONE_API_KEY is a good practice, but ensure that these keys are not logged or exposed in any way. Additionally, verify that the environment variables are securely managed.

    ⚡ Recommended focus areas for review

    Logging Warnings

    The new code introduces several logging warnings (e.g., _write_config() and _read_config() methods not implemented). These warnings should be reviewed to ensure they do not indicate potential issues or incomplete implementations.

        "2025-01-07 12:08:10 - semantic_router.utils.logger - WARNING - local.py:148 - _write_config() - No config is written for LocalIndex.\n"
       ]
      }
     ],
     "source": [
      "from semantic_router.routers import SemanticRouter\n",
      "\n",
      "local_rl =  SemanticRouter(encoder=encoder, routes=routes, index=local_index, auto_sync=\"local\")"
     ]
    },
    {
     "cell_type": "code",
     "execution_count": 11,
     "metadata": {},
     "outputs": [
      {
       "data": {
        "text/plain": [
         "['politics', 'chitchat']"
        ]
       },
       "execution_count": 11,
       "metadata": {},
       "output_type": "execute_result"
      }
     ],
     "source": [
      "local_rl.list_route_names()"
     ]
    },
    {
     "cell_type": "code",
     "execution_count": 12,
     "metadata": {},
     "outputs": [
      {
       "data": {
        "text/plain": [
         "'politics'"
        ]
       },
       "execution_count": 12,
       "metadata": {},
       "output_type": "execute_result"
      }
     ],
     "source": [
      "local_rl(\"don't you love politics?\").name"
     ]
    },
    {
     "cell_type": "code",
     "execution_count": 13,
     "metadata": {},
     "outputs": [
      {
       "data": {
        "text/plain": [
         "'chitchat'"
        ]
       },
       "execution_count": 13,
       "metadata": {},
       "output_type": "execute_result"
      }
     ],
     "source": [
      "local_rl(\"how's the weather today?\").name"
     ]
    },
    {
     "cell_type": "code",
     "execution_count": 14,
     "metadata": {},
     "outputs": [],
     "source": [
      "local_rl(\"I'm interested in learning about llama 2\").name"
     ]
    },
    {
     "cell_type": "markdown",
     "metadata": {},
     "source": [
      "We can delete or update routes."
     ]
    },
    {
     "cell_type": "code",
     "execution_count": 15,
     "metadata": {},
     "outputs": [
      {
       "data": {
        "text/plain": [
         "10"
        ]
       },
       "execution_count": 15,
       "metadata": {},
       "output_type": "execute_result"
      }
     ],
     "source": [
      "len(local_rl.index.index)"
     ]
    },
    {
     "cell_type": "code",
     "execution_count": 17,
     "metadata": {},
     "outputs": [
      {
       "name": "stderr",
       "output_type": "stream",
       "text": [
        "2025-01-07 12:08:29 - semantic_router.utils.logger - WARNING - base.py:172 - _read_config() - This method should be implemented by subclasses.\n",
        "2025-01-07 12:08:29 - semantic_router.utils.logger - WARNING - base.py:172 - _read_config() - This method should be implemented by subclasses.\n",
        "2025-01-07 12:08:29 - semantic_router.utils.logger - WARNING - base.py:823 - delete() - Route `chitchat` not found in SemanticRouter\n",
        "2025-01-07 12:08:29 - semantic_router.utils.logger - WARNING - local.py:148 - _write_config() - No config is written for LocalIndex.\n"
    
    Dependency Update

    The dependency update to semantic-router[pinecone]==0.1.0.dev3 should be validated for compatibility and stability, as it may introduce breaking changes or new issues.

    "!pip install -qU \"semantic-router[pinecone]==0.1.0.dev3\""
    
    API Key Handling

    The handling of OPENAI_API_KEY and PINECONE_API_KEY using getpass should be reviewed for security and usability, ensuring no sensitive information is exposed or mishandled.

      "os.environ[\"OPENAI_API_KEY\"] = os.environ.get(\"OPENAI_API_KEY\") or getpass(\n",
      "    \"Enter OpenAI API key: \"\n",
      ")\n",
      "encoder = OpenAIEncoder()"
     ]
    },
    {
     "cell_type": "code",
     "execution_count": 4,
     "metadata": {},
     "outputs": [
      {
       "name": "stderr",
       "output_type": "stream",
       "text": [
        "2025-01-07 12:10:29 - pinecone_plugin_interface.logging - INFO - discover_namespace_packages.py:12 - discover_subpackages() - Discovering subpackages in _NamespacePath(['c:\\\\Users\\\\Joshu\\\\OneDrive\\\\Documents\\\\Aurelio\\\\agents-course\\\\07-pratical-ai\\\\.venv\\\\Lib\\\\site-packages\\\\pinecone_plugins'])\n",
        "2025-01-07 12:10:29 - pinecone_plugin_interface.logging - INFO - discover_plugins.py:9 - discover_plugins() - Looking for plugins in pinecone_plugins.inference\n",
        "2025-01-07 12:10:29 - pinecone_plugin_interface.logging - INFO - installation.py:10 - install_plugins() - Installing plugin inference into Pinecone\n",
        "2025-01-07 12:10:31 - pinecone_plugin_interface.logging - INFO - discover_namespace_packages.py:12 - discover_subpackages() - Discovering subpackages in _NamespacePath(['c:\\\\Users\\\\Joshu\\\\OneDrive\\\\Documents\\\\Aurelio\\\\agents-course\\\\07-pratical-ai\\\\.venv\\\\Lib\\\\site-packages\\\\pinecone_plugins'])\n",
        "2025-01-07 12:10:31 - pinecone_plugin_interface.logging - INFO - discover_plugins.py:9 - discover_plugins() - Looking for plugins in pinecone_plugins.inference\n"
       ]
      }
     ],
     "source": [
      "from semantic_router.index.pinecone import PineconeIndex\n",
      "\n",
      "# get at app.pinecone.io\n",
      "os.environ[\"PINECONE_API_KEY\"] = os.environ.get(\"PINECONE_API_KEY\") or getpass(\n",
      "    \"Enter Pinecone API key: \"\n",
    

    Copy link

    github-actions bot commented Jan 7, 2025

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Score
    General
    Add error handling to the SemanticRouter initialization to improve robustness

    Add error handling for the SemanticRouter initialization to catch and log any issues
    related to the encoder, routes, or index setup.

    docs/indexes/pinecone.ipynb [109]

    -rl = SemanticRouter(encoder=encoder, routes=routes, index=pc_index, auto_sync="local")
    +try:
    +    rl = SemanticRouter(encoder=encoder, routes=routes, index=pc_index, auto_sync="local")
    +except Exception as e:
    +    print(f"Error initializing SemanticRouter: {e}")
    Suggestion importance[1-10]: 7

    Why: Adding error handling around the SemanticRouter initialization improves robustness by catching and logging potential issues. This is a reasonable enhancement, especially in cases where misconfiguration of the encoder, routes, or index could lead to runtime errors.

    7
    Adjust the auto_sync parameter to prevent unintended synchronization behavior

    Ensure that the auto_sync parameter in the SemanticRouter initialization is
    correctly configured to avoid potential synchronization issues or unexpected
    behavior.

    docs/indexes/local.ipynb [86]

    -local_rl =  SemanticRouter(encoder=encoder, routes=routes, index=local_index, auto_sync="local")
    +local_rl =  SemanticRouter(encoder=encoder, routes=routes, index=local_index, auto_sync="manual")
    Suggestion importance[1-10]: 5

    Why: The suggestion to change the auto_sync parameter from "local" to "manual" could be useful in preventing unintended synchronization issues. However, the PR does not indicate any specific problem with the current configuration, so the impact of this change is moderate and context-dependent.

    5

    Copy link

    codecov bot commented Jan 7, 2025

    Codecov Report

    All modified and coverable lines are covered by tests ✅

    Project coverage is 74.84%. Comparing base (9915053) to head (39bfcd9).
    Report is 1 commits behind head on main.

    Additional details and impacted files
    @@           Coverage Diff           @@
    ##             main     #502   +/-   ##
    =======================================
      Coverage   74.84%   74.84%           
    =======================================
      Files          43       43           
      Lines        3753     3753           
    =======================================
      Hits         2809     2809           
      Misses        944      944           

    ☔ View full report in Codecov by Sentry.
    📢 Have feedback on the report? Share it here.

    @Joshua-Briggs Joshua-Briggs linked an issue Jan 7, 2025 that may be closed by this pull request
    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.

    Update all Jupyter notebooks for v0.1.0
    1 participant