Skip to content

Commit

Permalink
[Opik-413] fix missing prompt data (#814)
Browse files Browse the repository at this point in the history
* Refactor metadata builder

* Add test for experiment.build_metadata_from_prompt_version
  • Loading branch information
alexkuzmik authored Dec 4, 2024
1 parent 8e3a40c commit 3925650
Show file tree
Hide file tree
Showing 5 changed files with 106 additions and 19 deletions.
3 changes: 2 additions & 1 deletion sdks/python/src/opik/api_objects/experiment/__init__.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
from .experiment import Experiment
from .helpers import build_metadata_and_prompt_version

__all__ = ["Experiment"]
__all__ = ["Experiment", "build_metadata_and_prompt_version"]
40 changes: 40 additions & 0 deletions sdks/python/src/opik/api_objects/experiment/helpers.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
from typing import Optional, Dict, Mapping, Tuple, Any
from .. import prompt
import logging
from opik import jsonable_encoder

LOGGER = logging.getLogger(__name__)


def build_metadata_and_prompt_version(
experiment_config: Optional[Dict[str, Any]], prompt: Optional[prompt.Prompt]
) -> Tuple[Optional[Dict[str, Any]], Optional[Dict[str, str]]]:
metadata = None
prompt_version: Optional[Dict[str, str]] = None

if experiment_config is None:
experiment_config = {}

if not isinstance(experiment_config, Mapping):
LOGGER.error(
"Experiment config must be dictionary, but %s was provided. Provided config will be ignored.",
experiment_config,
)
experiment_config = {}

if prompt is not None and "prompt" in experiment_config:
LOGGER.warning(
"The prompt parameter will not be added to experiment since there is already `prompt` specified in experiment_config"
)
return (experiment_config, None)

if prompt is not None:
prompt_version = {"id": prompt.__internal_api__version_id__}
experiment_config["prompt"] = prompt.prompt

if experiment_config == {}:
return None, None

metadata = jsonable_encoder.jsonable_encoder(experiment_config)

return metadata, prompt_version
22 changes: 4 additions & 18 deletions sdks/python/src/opik/api_objects/opik_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
import datetime
import logging

from typing import Optional, Any, Dict, List, Mapping
from typing import Optional, Any, Dict, List

from .prompt import Prompt
from .prompt.client import PromptClient
Expand All @@ -29,7 +29,6 @@
datetime_helpers,
config,
httpx_client,
jsonable_encoder,
url_helpers,
rest_client_configurator,
)
Expand Down Expand Up @@ -490,23 +489,10 @@ def create_experiment(
experiment.Experiment: The newly created experiment object.
"""
id = helpers.generate_id()
metadata = None
prompt_version: Optional[Dict[str, str]] = None

if isinstance(experiment_config, Mapping):
if prompt is not None:
prompt_version = {"id": prompt.__internal_api__version_id__}

if "prompt" not in experiment_config:
experiment_config["prompt"] = prompt.prompt

metadata = jsonable_encoder.jsonable_encoder(experiment_config)

elif experiment_config is not None:
LOGGER.error(
"Experiment config must be dictionary, but %s was provided. Config will not be logged.",
experiment_config,
)
metadata, prompt_version = experiment.build_metadata_and_prompt_version(
experiment_config=experiment_config, prompt=prompt
)

self._rest_client.experiments.create_experiment(
name=name,
Expand Down
Empty file.
60 changes: 60 additions & 0 deletions sdks/python/tests/unit/api_objects/experiment/test_helpers.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
from opik.api_objects import experiment
import pytest
import types


def fake_prompt():
return types.SimpleNamespace(
__internal_api__version_id__="some-prompt-version-id",
prompt="some-prompt-value",
)


@pytest.mark.parametrize(
argnames="input_kwargs,expected",
argvalues=[
(
{"experiment_config": None, "prompt": None},
{"metadata": None, "prompt_version": None},
),
(
{"experiment_config": {}, "prompt": None},
{"metadata": None, "prompt_version": None},
),
(
{"experiment_config": None, "prompt": fake_prompt()},
{
"metadata": {"prompt": "some-prompt-value"},
"prompt_version": {"id": "some-prompt-version-id"},
},
),
(
{"experiment_config": {}, "prompt": fake_prompt()},
{
"metadata": {"prompt": "some-prompt-value"},
"prompt_version": {"id": "some-prompt-version-id"},
},
),
(
{"experiment_config": {"some-key": "some-value"}, "prompt": None},
{"metadata": {"some-key": "some-value"}, "prompt_version": None},
),
(
{
"experiment_config": "NOT-DICT-VALUE-THAT-WILL-BE-IGNORED-AND-REPLACED-WITH-DICT-WITH-PROMPT",
"prompt": fake_prompt(),
},
{
"metadata": {"prompt": "some-prompt-value"},
"prompt_version": {"id": "some-prompt-version-id"},
},
),
],
)
def test_experiment_build_metadata_from_prompt_version(input_kwargs, expected):
metadata, prompt_version = experiment.build_metadata_and_prompt_version(
**input_kwargs
)

assert metadata == expected["metadata"]
assert prompt_version == expected["prompt_version"]

0 comments on commit 3925650

Please sign in to comment.