Skip to content

Commit

Permalink
Python: remove Verify and ./diagnostics (#287)
Browse files Browse the repository at this point in the history
### Motivation and Context

In an effort to make the Python port more idiomatic, let's remove the
`Verify` class and the `./diagnostics` sub-module. There are many more
follow-on tasks to do here, but this is a good start (and already a
large enough change).

### Description

This PR does the following:

1. Removes the `Verify` class, and re-writes all instances of `Verify.*`
2. Adds a `validation.py` in the `./utils` sub-module to hand some of
the more complex cases from `Verify` (checking that skills/functions/and
function params have valid names)
3. Removes the rest of the `./diagnostics` sub-module (w/ a longer-term
goal of removing all/most of our custom exception classes and, instead,
using appropriate built-in error classes)
  • Loading branch information
Jordan Henkel authored and dluc committed Apr 13, 2023
1 parent 407fb36 commit 1c2a505
Show file tree
Hide file tree
Showing 26 changed files with 190 additions and 282 deletions.
2 changes: 1 addition & 1 deletion python/.env.example
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
OPENAI_API_KEY=""
OPENAI_ORG_ID=""
AZURE_OPENAI_API_KEY=""
AZURE_OPENAI_ENDPOINT=""
AZURE_OPENAI_ENDPOINT=""
4 changes: 1 addition & 3 deletions python/semantic_kernel/ai/ai_exception.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,8 @@
from enum import Enum
from typing import Optional

from semantic_kernel.diagnostics.sk_exception import SKException


class AIException(SKException):
class AIException(Exception):
class ErrorCodes(Enum):
# Unknown error.
UnknownError = -1
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
from semantic_kernel.ai.open_ai.services.open_ai_chat_completion import (
OpenAIChatCompletion,
)
from semantic_kernel.diagnostics.verify import Verify


class AzureChatCompletion(OpenAIChatCompletion):
Expand All @@ -22,12 +21,14 @@ def __init__(
api_version: str = "2023-03-15-preview",
logger: Optional[Logger] = None,
) -> None:
Verify.not_empty(deployment_name, "You must provide a deployment name")
Verify.not_empty(api_key, "The Azure API key cannot be empty")
Verify.not_empty(endpoint, "The Azure endpoint cannot be empty")
Verify.starts_with(
endpoint, "https://", "The Azure endpoint must start with https://"
)
if not deployment_name:
raise ValueError("The deployment name cannot be `None` or empty")
if not api_key:
raise ValueError("The Azure API key cannot be `None` or empty`")
if not endpoint:
raise ValueError("The Azure endpoint cannot be `None` or empty")
if not endpoint.startswith("https://"):
raise ValueError("The Azure endpoint must start with https://")

self._endpoint = endpoint
self._api_version = api_version
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
from semantic_kernel.ai.open_ai.services.open_ai_text_completion import (
OpenAITextCompletion,
)
from semantic_kernel.diagnostics.verify import Verify


class AzureTextCompletion(OpenAITextCompletion):
Expand All @@ -22,12 +21,14 @@ def __init__(
api_version: str = "2022-12-01",
logger: Optional[Logger] = None,
) -> None:
Verify.not_empty(deployment_name, "You must provide a deployment name")
Verify.not_empty(api_key, "The Azure API key cannot be empty")
Verify.not_empty(endpoint, "The Azure endpoint cannot be empty")
Verify.starts_with(
endpoint, "https://", "The Azure endpoint must start with https://"
)
if not deployment_name:
raise ValueError("The deployment name cannot be `None` or empty")
if not api_key:
raise ValueError("The Azure API key cannot be `None` or empty`")
if not endpoint:
raise ValueError("The Azure endpoint cannot be `None` or empty")
if not endpoint.startswith("https://"):
raise ValueError("The Azure endpoint must start with https://")

self._endpoint = endpoint
self._api_version = api_version
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
from semantic_kernel.ai.open_ai.services.open_ai_text_embedding import (
OpenAITextEmbedding,
)
from semantic_kernel.diagnostics.verify import Verify


class AzureTextEmbedding(OpenAITextEmbedding):
Expand All @@ -22,12 +21,14 @@ def __init__(
api_version: str = "2022-12-01",
logger: Optional[Logger] = None,
) -> None:
Verify.not_empty(deployment_name, "You must provide a deployment name")
Verify.not_empty(api_key, "The Azure API key cannot be empty")
Verify.not_empty(endpoint, "The Azure endpoint cannot be empty")
Verify.starts_with(
endpoint, "https://", "The Azure endpoint must start with https://"
)
if not deployment_name:
raise ValueError("The deployment name cannot be `None` or empty")
if not api_key:
raise ValueError("The Azure API key cannot be `None` or empty`")
if not endpoint:
raise ValueError("The Azure endpoint cannot be `None` or empty")
if not endpoint.startswith("https://"):
raise ValueError("The Azure endpoint must start with https://")

self._endpoint = endpoint
self._api_version = api_version
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
from semantic_kernel.ai.ai_exception import AIException
from semantic_kernel.ai.chat_completion_client_base import ChatCompletionClientBase
from semantic_kernel.ai.chat_request_settings import ChatRequestSettings
from semantic_kernel.diagnostics.verify import Verify
from semantic_kernel.utils.null_logger import NullLogger


Expand Down Expand Up @@ -65,7 +64,8 @@ async def complete_chat_async(
Returns:
str -- The completed text.
"""
Verify.not_null(request_settings, "The request settings cannot be empty")
if request_settings is None:
raise ValueError("The request settings cannot be `None`")

if request_settings.max_tokens < 1:
raise AIException(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
from semantic_kernel.ai.ai_exception import AIException
from semantic_kernel.ai.complete_request_settings import CompleteRequestSettings
from semantic_kernel.ai.text_completion_client_base import TextCompletionClientBase
from semantic_kernel.diagnostics.verify import Verify
from semantic_kernel.utils.null_logger import NullLogger


Expand Down Expand Up @@ -65,8 +64,10 @@ async def complete_simple_async(
Returns:
str -- The completed text.
"""
Verify.not_empty(prompt, "The prompt is empty")
Verify.not_null(request_settings, "The request settings cannot be empty")
if not prompt:
raise ValueError("The prompt cannot be `None` or empty")
if request_settings is None:
raise ValueError("The request settings cannot be `None`")

if request_settings.max_tokens < 1:
raise AIException(
Expand Down
31 changes: 14 additions & 17 deletions python/semantic_kernel/core_skills/text_memory_skill.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
# Copyright (c) Microsoft. All rights reserved.

from semantic_kernel.diagnostics.verify import Verify
from semantic_kernel.orchestration.sk_context import SKContext
from semantic_kernel.skill_definition import sk_function, sk_function_context_parameter

Expand Down Expand Up @@ -44,19 +43,18 @@ async def recall_async(self, ask: str, context: SKContext) -> str:
Returns:
The nearest item from the memory store
"""
Verify.not_null(context.variables, "Context has no variables")
assert context.variables is not None # for type checker
Verify.not_null(context.memory, "Context has no memory")
assert context.memory is not None # for type checker
if context.variables is None:
raise ValueError("Context has no variables")
if context.memory is None:
raise ValueError("Context has no memory")

collection = (
context.variables[TextMemorySkill.COLLECTION_PARAM]
if context.variables.contains_key(TextMemorySkill.COLLECTION_PARAM)
else TextMemorySkill.DEFAULT_COLLECTION
)
Verify.not_empty(
collection, "Memory collection not defined for TextMemorySkill"
)
if not collection:
raise ValueError("Memory collection not defined for TextMemorySkill")

relevance = (
context.variables[TextMemorySkill.RELEVANCE_PARAM]
Expand Down Expand Up @@ -104,26 +102,25 @@ async def save_async(self, text: str, context: SKContext):
context -- Contains the 'collection' to save the information
and unique 'key' to associate with the information
"""
Verify.not_null(context.variables, "Context has no variables")
assert context.variables is not None # for type checker
Verify.not_null(context.memory, "Context has no memory")
assert context.memory is not None # for type checker
if context.variables is None:
raise ValueError("Context has no variables")
if context.memory is None:
raise ValueError("Context has no memory")

collection = (
context.variables[TextMemorySkill.COLLECTION_PARAM]
if context.variables.contains_key(TextMemorySkill.COLLECTION_PARAM)
else TextMemorySkill.DEFAULT_COLLECTION
)
Verify.not_empty(
collection, "Memory collection not defined for TextMemorySkill"
)
if not collection:
raise ValueError("Memory collection not defined for TextMemorySkill")

key = (
context.variables[TextMemorySkill.KEY_PARAM]
if context.variables.contains_key(TextMemorySkill.KEY_PARAM)
else None
)
Verify.not_empty(key, "Memory key not defined for TextMemorySkill")
assert key is not None # for type checker
if not key:
raise ValueError("Memory key not defined for TextMemorySkill")

await context.memory.save_information_async(collection, text=text, id=key)
29 changes: 0 additions & 29 deletions python/semantic_kernel/diagnostics/sk_exception.py

This file was deleted.

50 changes: 0 additions & 50 deletions python/semantic_kernel/diagnostics/validation_exception.py

This file was deleted.

105 changes: 0 additions & 105 deletions python/semantic_kernel/diagnostics/verify.py

This file was deleted.

Loading

0 comments on commit 1c2a505

Please sign in to comment.